On Thursday, 24 May 2018 16:01:22 CEST Richard W.M. Jones wrote:
> When the daemon starts up it creates a fresh (empty) LVM configuration
> and starts up lvmetad (which depends on the LVM configuration).
> 
> However this appears to cause problems: Some types of PV seem to
> require lvmetad and don't work without it
> (https://bugzilla.redhat.com/show_bug.cgi?id=1581810).  If we don't
> start lvmetad earlier, the device nodes are not created.
> 
> Therefore move the whole initialization step into appliance/init.
> 
> Two further changes had to be made:
> 
> Now we are using lvmetad all the time, using vgchange is incorrect.
> With lvmetad activated early we must use ‘pvscan --cache --activate ay’
> to scan all disks for PVs and activate any VGs on them (although the
> documentation is complex, confusing and contradictory so I'm not
> completely sure about this).
> 
> The ‘lvm_system_dir’ local variable in ‘daemon/lvm-filter.c’
> previously contained the path of the directory above $LVM_SYSTEM_DIR
> (eg. $LVM_SYSTEM_DIR = "/etc/lvm", lvm_system_dir = "/etc").  As this
> was highly confusing, I have changed it so the local variable and the
> environment variable have identical contents.  This involved removing
> the ‘lvm/’ component from a couple of paths since it is now included
> in the local variable.
> ---

Mostly LGTM, but there is one thing to fix:

> +  lvm_system_dir = getenv ("LVM_SYSTEM_DIR");
> +  if (!lvm_system_dir)
> +    lvm_system_dir = "/etc/lvm";
> +  fprintf (stderr, "lvm_system_dir = %s\n", lvm_system_dir);

The pointer returned by getenv() can change, so it will be better to
duplicate its result, and store that instead for the daemon lifetime.

-- 
Pino Toscano

Attachment: signature.asc
Description: This is a digitally signed message part.

_______________________________________________
Libguestfs mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libguestfs

Reply via email to