Libguestfs - Aug 2018 - Re: [PATCH] v2v: parse_libvirt_xml: handle srN CDROM devices (RHBZ#1612785)

This device naming is mostly written by virt-p2v, so get the slot from
it directly without using the drive_index "decoding" function.
---
 v2v/parse_libvirt_xml.ml | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/v2v/parse_libvirt_xml.ml b/v2v/parse_libvirt_xml.ml
index 36d2f66dd..55432f537 100644
--- a/v2v/parse_libvirt_xml.ml
+++ b/v2v/parse_libvirt_xml.ml
@@ -396,7 +396,15 @@ let parse_libvirt_xml ?conn xml                  else
                   loop rest
            in
-           loop ["hd"; "sd"; "vd";
"xvd"; "fd"] in
+           if String.is_prefix dev "sr" then (
+             let name = String.sub dev 2 (String.length dev - 2) in
+             try Some (int_of_string name)
+             with Failure _ ->
+               warning (f_"could not parse device name ‘%s’ from the
source libvirt XML") dev;
+               None
+           )
+           else
+             loop ["hd"; "sd"; "vd";
"xvd"; "fd"] in
 
       let typ          match xpath_string "@device" with
-- 
2.17.1

On Thu, Aug 09, 2018 at 03:05:26PM +0200, Pino Toscano
wrote:
> This device naming is mostly written by virt-p2v, so get the slot from
> it directly without using the drive_index "decoding" function.
> ---
>  v2v/parse_libvirt_xml.ml | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/v2v/parse_libvirt_xml.ml b/v2v/parse_libvirt_xml.ml
> index 36d2f66dd..55432f537 100644
> --- a/v2v/parse_libvirt_xml.ml
> +++ b/v2v/parse_libvirt_xml.ml
> @@ -396,7 +396,15 @@ let parse_libvirt_xml ?conn xml >                 
else
>                    loop rest
>             in
> -           loop ["hd"; "sd"; "vd";
"xvd"; "fd"] in
> +           if String.is_prefix dev "sr" then (
> +             let name = String.sub dev 2 (String.length dev - 2) in
> +             try Some (int_of_string name)
> +             with Failure _ ->
> +               warning (f_"could not parse device name ‘%s’ from the
source libvirt XML") dev;
> +               None
> +           )
> +           else
> +             loop ["hd"; "sd"; "vd";
"xvd"; "fd"] in
A bit less awkward would be:

   match target_dev with
   | None -> None
   | Some dev when String.is_prefix dev "sr" ->
       (* the additional code you've added above *)
   | Some dev ->
       (* the existing code *)

Conditional ACK on reworking the change to be like this.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

On Friday, 10 August 2018 14:51:12 CEST Richard W.M. Jones
wrote:
> On Thu, Aug 09, 2018 at 03:05:26PM +0200, Pino Toscano wrote:
> > This device naming is mostly written by virt-p2v, so get the slot from
> > it directly without using the drive_index "decoding"
function.
> > ---
> >  v2v/parse_libvirt_xml.ml | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/v2v/parse_libvirt_xml.ml b/v2v/parse_libvirt_xml.ml
> > index 36d2f66dd..55432f537 100644
> > --- a/v2v/parse_libvirt_xml.ml
> > +++ b/v2v/parse_libvirt_xml.ml
> > @@ -396,7 +396,15 @@ let parse_libvirt_xml ?conn xml > >        
else
> >                    loop rest
> >             in
> > -           loop ["hd"; "sd"; "vd";
"xvd"; "fd"] in
> > +           if String.is_prefix dev "sr" then (
> > +             let name = String.sub dev 2 (String.length dev - 2) in
> > +             try Some (int_of_string name)
> > +             with Failure _ ->
> > +               warning (f_"could not parse device name ‘%s’ from
the source libvirt XML") dev;
> > +               None
> > +           )
> > +           else
> > +             loop ["hd"; "sd"; "vd";
"xvd"; "fd"] in
> 
> A bit less awkward would be:
> 
>    match target_dev with
>    | None -> None
>    | Some dev when String.is_prefix dev "sr" ->
>        (* the additional code you've added above *)
>    | Some dev ->
>        (* the existing code *)
Good idea, thanks.
> Conditional ACK on reworking the change to be like this.
Amended with the above suggestion, and pushed.

-- 
Pino Toscano