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

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(a)redhat.com&gt; >> --- >> 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

Peter