[libvirt] PATCH: 3/5: The easy Xen and QEMU implementations

[Daniel Veillard]

On Wed, May 13, 2009 at 03:38:57PM +0100, Daniel P. Berrange wrote:
> This patches provides an implenmentation of the new APIs for Xen which
> can convert to/from both XM config format (/etc/xen files), and the
> SEXPR format used by XenD.  The former is most useful to end users, but
> it was easy to add the latter too, so I did. It can be a useful debugging
> aid sometimes. For QEMU, it implemnets export of  domain XML into the
> QEMU argv format.
> 
>  qemu_conf.c   |   25 +++++-------
>  qemu_conf.h   |    3 +
>  qemu_driver.c |  119 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  xen_unified.c |   95 +++++++++++++++++++++++++++++++++++++++++++++-
>  xen_unified.h |    3 +
>  5 files changed, 228 insertions(+), 17 deletions(-)
> 
> 
> Daniel
> 
> diff -r f55fa9b69d85 src/qemu_conf.c
> --- a/src/qemu_conf.c	Wed May 13 13:13:18 2009 +0100
> +++ b/src/qemu_conf.c	Wed May 13 13:16:50 2009 +0100
> @@ -1248,21 +1248,18 @@ int qemudBuildCommandLine(virConnectPtr 
>  
>              case VIR_DOMAIN_NET_TYPE_ETHERNET:
>                  {
> -                    char arg[PATH_MAX];
> -                    if (net->ifname) {
> -                        if (snprintf(arg, PATH_MAX-1, "tap,ifname=%s,script=%s,vlan=%d",
> -                                     net->ifname,
> -                                     net->data.ethernet.script,
> -                                     vlan) >= (PATH_MAX-1))
> -                            goto error;
> -                    } else {
> -                        if (snprintf(arg, PATH_MAX-1, "tap,script=%s,vlan=%d",
> -                                     net->data.ethernet.script,
> -                                     vlan) >= (PATH_MAX-1))
> -                            goto error;
> -                    }
> +                    virBuffer buf = VIR_BUFFER_INITIALIZER;
>  
> -                    ADD_ARG_LIT(arg);
> +                    virBufferAddLit(&buf, "tap");
> +                    if (net->ifname)
> +                        virBufferVSprintf(&buf, ",ifname=%s", net->ifname);
> +                    if (net->data.ethernet.script)
> +                        virBufferVSprintf(&buf, ",script=%s", net->data.ethernet.script);
> +                    virBufferVSprintf(&buf, ",vlan=%d", vlan);
> +                    if (virBufferError(&buf))
> +                        goto error;
> +
> +                    ADD_ARG(virBufferContentAndReset(&buf));
>                  }
>                  break;

  Hum, that part looks a bit orthogonal to the patch itself, isn't it ?

[...]
> +    /* Since we're just exporting args, we can't do bridge/network
> +     * setups, since libvirt will normally create TAP devices
> +     * directly. We convert those configs into generic 'ethernet'
> +     * config and assume the user has suitable 'ifup-qemu' scripts
> +     */
> +    for (i = 0 ; i < def->nnets ; i++) {
> +        virDomainNetDefPtr net = def->nets[i];
> +        if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
> +            VIR_FREE(net->data.network.name);
> +            net->type = VIR_DOMAIN_NET_TYPE_ETHERNET;
> +            net->data.ethernet.dev = NULL;
> +            net->data.ethernet.script = NULL;
> +            net->data.ethernet.ipaddr = NULL;
> +        } else if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
> +            net->type = VIR_DOMAIN_NET_TYPE_ETHERNET;
> +            /* Rely on fact that the 'ethernet' and 'bridge'
> +             * union structs have members in same place */

   Hum, it's a bit fishy... why not play safe, shaving a couple
   microseconds here is probably not worth it.

[...]
> +cleanup:
> +    for (tmp = retargv ; tmp && *tmp ; tmp++)

  weird, we use a static string just for "" ?

> +        VIR_FREE(*tmp);
> +    VIR_FREE(retargv);
> +
> +    for (tmp = retenv ; tmp && *tmp ; tmp++)

  same

> +        VIR_FREE(*tmp);
> +    VIR_FREE(retenv);

[...]
> --- a/src/xen_unified.h	Wed May 13 13:13:18 2009 +0100
> +++ b/src/xen_unified.h	Wed May 13 13:16:50 2009 +0100
> @@ -46,6 +46,9 @@ extern int xenRegister (void);
>  
>  #define MIN_XEN_GUEST_SIZE 64  /* 64 megabytes */
>  
> +#define XEN_CONFIG_FORMAT_XM    "xen-xm"
> +#define XEN_CONFIG_FORMAT_SEXPR "xen-sxpr"

  Actually, shouldn't those be part of the public headers instead
I'm afraid I'm missing something here ...

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/