[libvirt] [PATCH v2 2/8] qemu: Implement chardev hotplug on config level

[Martin Kletzander]

On 06/06/2013 02:29 PM, 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 what this patch
> exactly does. There is currently no implementation for a chardev

s/what this patch exactly does/exactly what this patch does/

> 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.
> ---
>  src/conf/domain_conf.c | 22 ++++++++++++++++++++--
>  src/qemu/qemu_driver.c | 33 ++++++++++++++++++++++++++++++---
>  2 files changed, 50 insertions(+), 5 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 37f0ce9..582eade 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1755,10 +1755,12 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def)
>      case VIR_DOMAIN_DEVICE_RNG:
>          virDomainRNGDefFree(def->data.rng);
>          break;
> +    case VIR_DOMAIN_DEVICE_CHR:
> +        virDomainChrDefFree(def->data.chr);
> +        break;
>      case VIR_DOMAIN_DEVICE_NONE:
>      case VIR_DOMAIN_DEVICE_FS:
>      case VIR_DOMAIN_DEVICE_SMARTCARD:
> -    case VIR_DOMAIN_DEVICE_CHR:
>      case VIR_DOMAIN_DEVICE_MEMBALLOON:
>      case VIR_DOMAIN_DEVICE_NVRAM:
>      case VIR_DOMAIN_DEVICE_LAST:
> @@ -9433,6 +9435,20 @@ virDomainDeviceDefParse(const char *xmlStr,
>          dev->type = VIR_DOMAIN_DEVICE_RNG;
>          if (!(dev->data.rng = virDomainRNGDefParseXML(node, ctxt, flags)))
>              goto error;
> +    } else if (xmlStrEqual(node->name, BAD_CAST "channel") ||
> +               xmlStrEqual(node->name, BAD_CAST "console") ||
> +               xmlStrEqual(node->name, BAD_CAST "parallel") ||
> +               xmlStrEqual(node->name, BAD_CAST "serial")) {
> +        dev->type = VIR_DOMAIN_DEVICE_CHR;
> +        /* In case of serial chardev we want to parse <source> as it is the only
> +         * thing distinguishing two serial chardevs */

The comment has no information value, we have to parse the XML anyway.

> +        flags &= ~VIR_DOMAIN_XML_INACTIVE;

You're silently removing the flag without telling the user that.  It's
ok if you remove that in the last patch (which I haven't seen yet), but
I guess error-ing out would be more appropriate.

ACK,

Martin