[libvirt] [PATCH v4 2/9] qemu: Implement chardev hotplug on config level

[Daniel P. Berrange]

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 :|