On Wed, Jul 10, 2013 at 07:02:52PM +0200, Michal Privoznik wrote:
> There are two levels on which a device may be hotplugged: config
> and live. The config level requires just an insert or remove from
> internal domain definition structure, which is exactly what this
> patch does. There is currently no implementation for a chardev
> update action, as there's not much to be updated. But more
> importantly, the only thing that can be updated is path or socket
> address by which chardevs are distinguished. So the update action
> is currently not supported.
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index b0180c9..d858131 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -6607,6 +6607,7 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
> virDomainHostdevDefPtr hostdev;
> virDomainLeaseDefPtr lease;
> virDomainControllerDefPtr controller;
> + virDomainChrDefPtr chr;
>
> switch (dev->type) {
> case VIR_DOMAIN_DEVICE_DISK:
> @@ -6682,10 +6683,23 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
> return -1;
> break;
>
> + case VIR_DOMAIN_DEVICE_CHR:
> + chr = dev->data.chr;
> + if (virDomainChrFind(vmdef, chr)) {
> + virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> + _("chardev already exists"));
> + return -1;
> + }
> +
> + if (virDomainChrInsert(vmdef, chr) < 0)
> + return -1;
> + dev->data.chr = NULL;
> + break;
Hmm, this is unconditionally adding the device to the list, which in
general is fine..... except for the magic serial/console duplication.
If we have zero serial devices + zero console devices, and then
hotplug a serial device, we must duplicate that as the first console
device.
We should also refuse to allow you to hotplug a <console> element
with target type == serial.
> @@ -6767,6 +6782,18 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
>
> break;
>
> + case VIR_DOMAIN_DEVICE_CHR:
> + if (!(chr = virDomainChrRemove(vmdef, dev->data.chr))) {
> + virReportError(VIR_ERR_INVALID_ARG, "%s",
> + _("device not present in domain configuration"));
> + return -1;
> + }
> +
> + virDomainChrDefFree(chr);
> + virDomainChrDefFree(dev->data.chr);
> + dev->data.chr = NULL;
> + break;
And if we remove the first serial device here, then we also need to
remove the compat console device with targettype==serial
And probably ought to forbid removing a <console> with type=serial,
instead requiring them to remove the <serial> device instead.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list