[Libvir] [PATCH] Allow modifying existing block device, making virDomainAttachDevice modify the device if it already exists

[Daniel Veillard]

On Thu, Sep 06, 2007 at 11:45:39AM -0400, Hugh Brock wrote:
> The attached patch adds code to xend_internal.c:xenDomainAttachDevice 
> that checks to see if the device mentioned in the passed-in XML already 
> exists, and if so calls op_device_configure to modify the device. It is 
> particularly useful for connecting and disconnecting a hardware cdrom 
> device to an FV guest.
> 
> I considered making new API a la virDomainModifyDevice, but decided 
> overloading virDomainAttachDevice was cleaner (and didn't change 
> existing API).
> 
> I have tested the patch on f7 with xen 3.1 and a windows 2000 guest. 
> Since the patch merely wraps Xen's device_configure call, we're not 
> adding much risk of breakage.

  Okay looks sensible. 
> diff -ruN libvirt.orig/src/xend_internal.c libvirt/src/xend_internal.c
> --- libvirt.orig/src/xend_internal.c	2007-09-06 11:28:05.000000000 -0400
> +++ libvirt/src/xend_internal.c	2007-09-06 10:47:25.000000000 -0400
> @@ -3087,6 +3087,7 @@
>      char *sexpr, *conf, *str;
>      int hvm = 0, ret;
>      xenUnifiedPrivatePtr priv;
> +    char class[8], ref[80];
>  
>      if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) {
>          virXendError((domain ? domain->conn : NULL), VIR_ERR_INVALID_ARG,
> @@ -3116,8 +3117,16 @@
>          *(conf + strlen(conf) -1) = 0; /* suppress final ) */
>      }
>      else conf = sexpr;
> -    ret = xend_op(domain->conn, domain->name, "op", "device_create",
> -        "config", conf, NULL);
> +    if (virDomainXMLDevID(domain, xml, class, ref)) {
> +        /* device doesn't exist, define it */
> +        ret = xend_op(domain->conn, domain->name, "op", "device_create",
> +                      "config", conf, NULL);
> +    } 
> +    else {
> +        /* device exists, attempt to modify it */
> +        ret = xend_op(domain->conn, domain->name, "op", "device_configure", 
> +                      "config", conf, "dev", ref, NULL);
> +    }
>      free(sexpr);
>      return ret;
>  }

  virDomainXMLDevID looks frightening to me since we write to an array of
undisclosed size, but that is independant from the patch which looks fine to
me.

  +1

  Thanks !

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard at redhat.com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/