Code review comment for ~redriver/cloud-init:frbsd-azure-branch

Scott Moser (smoser) wrote :

> > There are some things that still need fixing though, and I have some in-line
> > comments to help elaborate.
> > a.) You've fixed some general things inside of the DataSourceAzureNet only.
> > Ie, you made devent2deev run through the datasource, which means
> > that that code wont work running freebsd on Ec2. Better to have
> > logic in it that does the right thing both places.
> When I change devent2dev, I have considered other platforms. I have put a
> generic implementation of get_mount_info in sources/ So, for other
> platform, for example, EC2, it will invoke the util.get_mount_info. See:
> def get_mount_info(self, path, log=LOG):
> return util.get_mount_info(path, log)

devent2dev basically is supposed to turn either a filesystem location ("/home")
or a device into a device that contains that filesystem. Input of '/dev/sda'
should return '/dev/sda'. Input of "/home" should return the device that
/home's filesystem is on (what should be rezied).

Your made a change to resize_devices in cloudinit/config/ to
take a 'cloud'.

  - resize_devices then calls devent2dev(devent, cloud)
  - devent2dev calls cloud.datasource.get_mount_info(devent)
This change:
- result = util.get_mount_info(devent)
+ result = cloud.datasource.get_mount_info(devent)

I'd have preferred for 'util.get_mount_info' to have code that says:
   if is_FreeBSD:
      # get the device one way
      # get it for linux.

Instead, since you're going through 'datasource', your fix will only work
on Azure.

> > c.) is your '_can_skip_resize_ufs' for performance? or is it actually
> needed?
> It is used to check whether resize is necessary. On Azure, the partition has
> already fully covered the whole disk. It will report error in log if we resize
> it by force. This pre-check wants to avoid such error.

OK, thats fine.

« Back to merge proposal