[Libguestfs] [PATCH] daemon: inspect_fs_windows: Handle parted errors

[Sam Eiderman]

Gentle ping

On Tue, Jun 30, 2020, 14:25 Sam Eiderman <sameid at google.com> wrote:

> Yea, I noticed that commit, but since it was used in gpt too
> regardless of that commit, I decided not to mention this regression.
>
> I also noticed that list_filesystems() still works, this is probably
> due to filtering of devices (daemon/listfs.ml):
>
> "let devices = List.filter is_not_partitioned_device devices in"
>
> What do you have in mind here?
>
> On Tue, Jun 30, 2020 at 12:33 PM Pino Toscano <ptoscano at redhat.com> wrote:
> >
> > On Tuesday, 30 June 2020 10:33:40 CEST Sam Eiderman wrote:
> > > By creating an empty disk and using it as the first disk of the vm
> (i.e.
> > > /dev/sda, /dev/sdb{1,2} contains the windows fses) we change the
> > > iteration order of the disks.
> > > This causes inspect_os() to fail since Parted returns a Unix_error if
> > > the device does not contain any partitions - fix this by handling this
> > > Unix_error.
> > >
> > > Signed-off-by: Sam Eiderman <sameid at google.com>
> > > ---
> > >  daemon/inspect_fs_windows.ml | 11 ++++++++---
> > >  1 file changed, 8 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/daemon/inspect_fs_windows.ml b/daemon/
> inspect_fs_windows.ml
> > > index c4a05bc38..bc6b98b60 100644
> > > --- a/daemon/inspect_fs_windows.ml
> > > +++ b/daemon/inspect_fs_windows.ml
> > > @@ -365,8 +365,10 @@ and map_registry_disk_blob_mbr devices blob =
> > >      let device =
> > >        List.find (
> > >          fun dev ->
> > > -          Parted.part_get_parttype dev = "msdos" &&
> > > -            pread dev 4 0x01b8 = diskid
> > > +          try
> > > +            Parted.part_get_parttype dev = "msdos" &&
> > > +              pread dev 4 0x01b8 = diskid
> > > +          with Unix.Unix_error (Unix.EINVAL, _, _) -> false
> >
> > The old C inspection code did not check for the partition type:
> >
> https://github.com/libguestfs/libguestfs/blob/stable-1.36/lib/inspect-fs-windows.c#L591
> > The partition type check was added recently:
> >
> https://github.com/libguestfs/libguestfs/commit/7b4d13626ff878b124d01ec452a4fd0052934133
> >
> > TBH I'd rather prefer that we check for the situation explicitly
> > before, rather than catching a generic EINVAL error. Or, even better,
> > try to avoid altogether the situation that 7b4d13626ff878 checks,
> > handling this issue as well.
> >
> > > @@ -402,7 +404,10 @@ and map_registry_disk_blob_gpt partitions blob =
> > >          fun part ->
> > >            let partnum = Devsparts.part_to_partnum part in
> > >            let device = Devsparts.part_to_dev part in
> > > -          let typ = Parted.part_get_parttype device in
> > > +          let typ =
> > > +            try
> > > +              Parted.part_get_parttype device
> > > +            with Unix.Unix_error (Unix.EINVAL, _, _) -> "unknown" in
> > >            if typ <> "gpt" then false
> > >            else (
> >
> > Ditto.
> >
> > --
> > Pino Toscano_______________________________________________
> > Libguestfs mailing list
> > Libguestfs at redhat.com
> > https://www.redhat.com/mailman/listinfo/libguestfs
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20200922/20cdf1e3/attachment.htm>