Libguestfs - Aug 2021 - [PATCH virt-v2v 1/2] v2v: -o rhv-upload: Generate disk UUIDs

On Tue, Aug 03, 2021 at 06:06:26PM +0300, Nir Soffer
wrote:
> On Tue, Aug 3, 2021 at 12:26 PM Richard W.M. Jones <rjones at
redhat.com> wrote:
> > diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py
> > index a3d578176..1155cf38d 100644
> > --- a/v2v/rhv-upload-plugin.py
> > +++ b/v2v/rhv-upload-plugin.py
> > @@ -482,8 +482,7 @@ def create_disk(connection):
> >
> >      disk = disks_service.add(
> >          disk=types.Disk(
> > -            # The ID is optional.
> > -            id=params.get('rhv_disk_uuid'),
> > +            id=params.get('disk_uuid'),
> 
> The disk id is not optional now. If it is not specified in params, the
> caller has no way
> to get the id generated by RHV. So better remove the comment and use:
> 
>     params["disk_id"]
> 
> This will fail early in case there is a bug in the caller code.
Not sure I understand.  Isn't that what this hunk does?
> >              name=params['disk_name'],
> >              description="Uploaded by virt-v2v",
> >              format=disk_format,
> > diff --git a/v2v/rhv-upload-precheck.py b/v2v/rhv-upload-precheck.py
> > index c3f74df35..1dc1b8498 100644
> > --- a/v2v/rhv-upload-precheck.py
> > +++ b/v2v/rhv-upload-precheck.py
> > @@ -98,6 +98,8 @@ if cpu.architecture == types.Architecture.UNDEFINED:
> >                         (params['rhv_cluster']))
> >
> >  # Find if any disk already exists with specified UUID.
> > +# Only used with -oo rhv-disk-uuid.  It is assumed that the
> > +# random UUIDs that we generate are unlikely to conflict.
> >  disks_service = system_service.disks_service()
> >
> >  for uuid in params.get('rhv_disk_uuids', []):
> > --
> > 2.32.0
> >
> 
> I think we should also remove the code creating temporary file with
> the disk uuid.
Ooops, that was left in there by accident (and removed in
a later commit).

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

On Tue, Aug 3, 2021 at 6:37 PM Richard W.M. Jones <rjones at redhat.com>
wrote:
>
> On Tue, Aug 03, 2021 at 06:06:26PM +0300, Nir Soffer wrote:
> > On Tue, Aug 3, 2021 at 12:26 PM Richard W.M. Jones <rjones at
redhat.com> wrote:
> > > diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py
> > > index a3d578176..1155cf38d 100644
> > > --- a/v2v/rhv-upload-plugin.py
> > > +++ b/v2v/rhv-upload-plugin.py
> > > @@ -482,8 +482,7 @@ def create_disk(connection):
> > >
> > >      disk = disks_service.add(
> > >          disk=types.Disk(
> > > -            # The ID is optional.
> > > -            id=params.get('rhv_disk_uuid'),
> > > +            id=params.get('disk_uuid'),
> >
> > The disk id is not optional now. If it is not specified in params, the
> > caller has no way
> > to get the id generated by RHV. So better remove the comment and use:
> >
> >     params["disk_id"]
> >
> > This will fail early in case there is a bug in the caller code.
>
> Not sure I understand.  Isn't that what this hunk does?'
params.get("disk_uuid") will return None silently, then RHV will
create
a disk with a new random UUID.

We want to fail loudly with a KeyError in this case.