On Fri, Oct 09, 2020 at 02:21:09PM +0100, Richard W.M. Jones wrote:
On Fri, Oct 09, 2020 at 11:37:41AM +0100, Richard W.M. Jones wrote:On Fri, Oct 09, 2020 at 11:33:43AM +0100, Richard W.M. Jones wrote: > > This is the patch I tested which works (on top of the > patch posted): > > diff --git a/lib/canonical-name.c b/lib/canonical-name.c > index e0c7918b4..ae4def692 100644 > --- a/lib/canonical-name.c > +++ b/lib/canonical-name.c > @@ -53,8 +53,16 @@ guestfs_impl_canonical_device_name (guestfs_h *g, const char *device) > * BitLocker-encrypted volume, so simply return the original > * name in that case. > */ > - if (ret == NULL && guestfs_last_errno (g) == EINVAL) > - ret = safe_strdup (g, device); > + if (ret == NULL) { > + if (guestfs_last_errno (g) == EINVAL) > + ret = safe_strdup (g, device); > + else > + /* Make sure the original error gets pushed through the > + * error handlers. > + */ > + guestfs_int_error_errno (g, guestfs_last_errno (g), > + "%s", guestfs_last_error (g)); > + } > } > else > ret = safe_strdup (g, device); > > --- > > Current upstream: > > $ guestfish scratch 1M : run : canonical-device-name "/dev/dm-999" > libguestfs: error: lvm_canonical_lv_name: stat: /dev/dm-999: No such file or directory > /dev/dm-999 <----- > > Patch posted without the above patch added: > > $ ./run guestfish scratch 1M : run : canonical-device-name "/dev/dm-999" > > (no output, but the command fails with exit code 1) > > Patch posted + above patch: > > $ ./run guestfish scratch 1M : run : canonical-device-name "/dev/dm-999" > libguestfs: error: lvm_canonical_lv_name: stat: /dev/dm-999: No such file or directoryActually I didn't notice this, but it improves on the current upstream behaviour. Current upstream calls the error handlers and returns a non-error original string (see <----- line above) and exit code 0. With the patch we return an error.This breaks virt-inspector, at least when we run the test suite which has a phony Ubuntu guest with a non-existent /dev/mapper/* in its /etc/fstab. The manpage for guestfs_canonical_device_name[1] is sort of ambiguous here. It says that "/dev/mapper/*" is Converted to /dev/VG/LV form using guestfs_lvm_canonical_lv_name. and guestfs_lvm_canonical_lv_name will certainly return an error for a non-existent name. However it does also say: Other strings are returned unmodified. You can sort of read it both ways. Because it breaks a long-standing user of this API I'm going to change this so it behaves more like the current upstream code (original error goes to debug, return original device string).
OK, that makes sense.
Rich. [1] https://libguestfs.org/guestfs.3.html#guestfs_canonical_device_name -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
signature.asc
Description: PGP signature
_______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
