Re: [PATCH 06/12] qemu: add id when build RNG device and rename object id

[lhuang <lhuang@xxxxxxxxxx>]

On 01/05/2015 11:45 PM, Peter Krempa wrote:
> On 01/05/15 15:51, Peter Krempa wrote:
> > On 01/03/15 06:06, Luyao Huang wrote:
> > > We didn't set a id when we build RNG device cmdline before.
> > > Give a id to every RNG device and we can hotunplug it via
> > > QMP cmd device_del.
> > >
> > > Signed-off-by: Luyao Huang <lhuang@xxxxxxxxxx>
> > > ---
> > >   src/qemu/qemu_command.c | 12 ++++++------
> > >   1 file changed, 6 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > > index 46e289d..a4073ee 100644
> > > --- a/src/qemu/qemu_command.c
> > > +++ b/src/qemu/qemu_command.c
> > > @@ -5761,7 +5761,7 @@ qemuBuildRNGBackendArgs(virCommandPtr cmd,
> > >               goto cleanup;
> > >           }
> > >   
> > > -        virBufferAsprintf(&buf, "rng-random,id=%s,filename=%s",
> > > +        virBufferAsprintf(&buf, "rng-random,id=obj%s,filename=%s",
> > >                             dev->info.alias, dev->source.file);
> > >   
> > >           virCommandAddArg(cmd, "-object");
> > > @@ -5784,7 +5784,7 @@ qemuBuildRNGBackendArgs(virCommandPtr cmd,
> > >           virCommandAddArgList(cmd, "-chardev", backend, NULL);
> > >   
> > >           virCommandAddArg(cmd, "-object");
> > > -        virCommandAddArgFormat(cmd, "rng-egd,chardev=char%s,id=%s",
> > > +        virCommandAddArgFormat(cmd, "rng-egd,chardev=char%s,id=obj%s",
> > >                                  dev->info.alias, dev->info.alias);
> > >           break;
> > >   
> > > @@ -5816,13 +5816,13 @@ qemuBuildRNGDevStr(virDomainDefPtr def,
> > >       }
> > >   
> > >       if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)
> > > -        virBufferAsprintf(&buf, "virtio-rng-ccw,rng=%s", dev->info.alias);
> > > +        virBufferAsprintf(&buf, "virtio-rng-ccw,rng=obj%s,id=%s", dev->info.alias, dev->info.alias);
> > >       else if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390)
> > > -        virBufferAsprintf(&buf, "virtio-rng-s390,rng=%s", dev->info.alias);
> > > +        virBufferAsprintf(&buf, "virtio-rng-s390,rng=obj%s,id=%s", dev->info.alias, dev->info.alias);
> > >       else if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO)
> > > -        virBufferAsprintf(&buf, "virtio-rng-device,rng=%s", dev->info.alias);
> > > +        virBufferAsprintf(&buf, "virtio-rng-device,rng=obj%s,id=%s", dev->info.alias, dev->info.alias);
> > >       else
> > > -        virBufferAsprintf(&buf, "virtio-rng-pci,rng=%s", dev->info.alias);
> > > +        virBufferAsprintf(&buf, "virtio-rng-pci,rng=obj%s,id=%s", dev->info.alias, dev->info.alias);
> > >   
> > >       if (dev->rate > 0) {
> > >           virBufferAsprintf(&buf, ",max-bytes=%u", dev->rate);
> >
> > This breaks the testsuite as you didn't fix the expected outputs after
> > such a change. Please always run "make check" before posting patches.
> >
> > Additional question. Is this change even necessary? The RNG device does
> > have it's alias already whithout the "obj" prefix ... thus it's just
> > "rng0" for example.
> I misread the code. This is actually necessary as otherwise the -device
> would have the same ID as the backend object. That makes sense to
> change, although we need to make sure then that the code will work in
> case of a long running VM (with the incorrect name) and a new libvirt
> instance.
>
> At any rate ... you need to fix the tests after this commit
Thanks for your review.
Yes, I will give another commit for the tests fix in next version.

I have test with a long running VM (start in old libvirt which RNG 
device no
device name), and update to a libvirt which have these code, if we try 
to hot-unplug the
rng device, qemu will return a error like this :
internal error: unable to execute QEMU command 'device_del': Device 
'rng0' not found

maybe my qemu is too old ? vm qemu cmdline -device do not have a id 
(because i start it in the
old libvirt), so this error is correct in this place. But i don't know 
how can i hot-unplug a device without
a id.
> Peter
Luyao

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list