January 2009 - Devel - libvirt List Archives

[libvirt] [PATCH] Parse ipaddr for bridge network types
by john.levon＠sun.com 19 Jan '09

19 Jan '09

# HG changeset patch # User john.levon(a)sun.com # Date 1232054247 28800 # Node ID 2b8a53968637c1ba806a1cdd94a24f4d15f8072a # Parent 04139c088854c23dc548c0f9f4abf54c4ed07e0d Parse ipaddr for bridge network types As well as supporting round-tripping of the IP address, this fixes a leak if ipaddr was specified in the Xen domain config. Signed-off-by: John Levon <john.levon(a)sun.com> diff --git a/src/domain_conf.c b/src/domain_conf.c --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -277,6 +277,7 @@ void virDomainNetDefFree(virDomainNetDef case VIR_DOMAIN_NET_TYPE_BRIDGE: VIR_FREE(def->data.bridge.brname); VIR_FREE(def->data.bridge.script); + VIR_FREE(def->data.bridge.ipaddr); break; } @@ -867,7 +868,8 @@ virDomainNetDefParseXML(virConnectPtr co address = virXMLPropString(cur, "address"); port = virXMLPropString(cur, "port"); } else if ((address == NULL) && - (def->type == VIR_DOMAIN_NET_TYPE_ETHERNET) && + (def->type == VIR_DOMAIN_NET_TYPE_ETHERNET || + def->type == VIR_DOMAIN_NET_TYPE_BRIDGE) && (xmlStrEqual(cur->name, BAD_CAST "ip"))) { address = virXMLPropString(cur, "address"); } else if ((ifname == NULL) && @@ -933,6 +935,10 @@ virDomainNetDefParseXML(virConnectPtr co if (script != NULL) { def->data.bridge.script = script; script = NULL; + } + if (address != NULL) { + def->data.bridge.ipaddr = address; + address = NULL; } break; @@ -2870,6 +2876,9 @@ virDomainNetDefFormat(virConnectPtr conn case VIR_DOMAIN_NET_TYPE_BRIDGE: virBufferEscapeString(buf, " <source bridge='%s'/>\n", def->data.bridge.brname); + if (def->data.bridge.ipaddr) + virBufferVSprintf(buf, " <ip address='%s'/>\n", + def->data.bridge.ipaddr); if (def->data.bridge.script) virBufferEscapeString(buf, " <script path='%s'/>\n", def->data.bridge.script); diff --git a/src/domain_conf.h b/src/domain_conf.h --- a/src/domain_conf.h +++ b/src/domain_conf.h @@ -154,6 +154,7 @@ struct _virDomainNetDef { struct { char *brname; char *script; + char *ipaddr; } bridge; } data; char *ifname; diff --git a/src/xend_internal.c b/src/xend_internal.c --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -1792,10 +1792,18 @@ xenDaemonParseSxprNets(virConnectPtr con net->type == VIR_DOMAIN_NET_TYPE_BRIDGE && !(net->data.bridge.script = strdup(tmp2))) goto no_memory; + tmp = sexpr_node(node, "device/vif/ip"); + if (tmp && + !(net->data.bridge.ipaddr = strdup(tmp))) + goto no_memory; } else { net->type = VIR_DOMAIN_NET_TYPE_ETHERNET; if (tmp2 && !(net->data.ethernet.script = strdup(tmp2))) + goto no_memory; + tmp = sexpr_node(node, "device/vif/ip"); + if (tmp && + !(net->data.ethernet.ipaddr = strdup(tmp))) goto no_memory; } @@ -1829,11 +1837,6 @@ xenDaemonParseSxprNets(virConnectPtr con net->mac[4] = mac[4]; net->mac[5] = mac[5]; } - - tmp = sexpr_node(node, "device/vif/ip"); - if (tmp && - !(net->data.ethernet.ipaddr = strdup(tmp))) - goto no_memory; if (model && !(net->model = strdup(model))) @@ -5128,6 +5131,8 @@ xenDaemonFormatSxprNet(virConnectPtr con script = def->data.bridge.script; virBufferVSprintf(buf, "(script '%s')", script); + if (def->data.bridge.ipaddr != NULL) + virBufferVSprintf(buf, "(ip '%s')", def->data.bridge.ipaddr); break; case VIR_DOMAIN_NET_TYPE_NETWORK: diff --git a/src/xm_internal.c b/src/xm_internal.c --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -1086,6 +1086,9 @@ xenXMDomainConfigParse(virConnectPtr con if (script[0] && !(net->data.bridge.script = strdup(script))) goto no_memory; + if (ip[0] && + !(net->data.bridge.ipaddr = strdup(ip))) + goto no_memory; } else { if (script[0] && !(net->data.ethernet.script = strdup(script))) @@ -1822,6 +1825,8 @@ static int xenXMDomainConfigFormatNet(vi switch (net->type) { case VIR_DOMAIN_NET_TYPE_BRIDGE: virBufferVSprintf(&buf, ",bridge=%s", net->data.bridge.brname); + if (net->data.bridge.ipaddr) + virBufferVSprintf(&buf, ",ip=%s", net->data.bridge.ipaddr); break; case VIR_DOMAIN_NET_TYPE_ETHERNET:

2 1

[libvirt] [PATCH] patch queue: bridge-script
by john.levon＠sun.com 19 Jan '09

19 Jan '09

# HG changeset patch # User john.levon(a)sun.com # Date 1232052415 28800 # Node ID 04139c088854c23dc548c0f9f4abf54c4ed07e0d # Parent 253da3acb103a4ce764cb8192a65a4d51833c2f1 patch queue: bridge-script diff --git a/src/domain_conf.c b/src/domain_conf.c --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -276,6 +276,7 @@ void virDomainNetDefFree(virDomainNetDef case VIR_DOMAIN_NET_TYPE_BRIDGE: VIR_FREE(def->data.bridge.brname); + VIR_FREE(def->data.bridge.script); break; } @@ -877,7 +878,8 @@ virDomainNetDefParseXML(virConnectPtr co VIR_FREE(ifname); } } else if ((script == NULL) && - (def->type == VIR_DOMAIN_NET_TYPE_ETHERNET) && + (def->type == VIR_DOMAIN_NET_TYPE_ETHERNET || + def->type == VIR_DOMAIN_NET_TYPE_BRIDGE) && xmlStrEqual(cur->name, BAD_CAST "script")) { script = virXMLPropString(cur, "path"); } else if (xmlStrEqual (cur->name, BAD_CAST "model")) { @@ -928,6 +930,10 @@ virDomainNetDefParseXML(virConnectPtr co } def->data.bridge.brname = bridge; bridge = NULL; + if (script != NULL) { + def->data.bridge.script = script; + script = NULL; + } break; case VIR_DOMAIN_NET_TYPE_CLIENT: @@ -2864,6 +2870,9 @@ virDomainNetDefFormat(virConnectPtr conn case VIR_DOMAIN_NET_TYPE_BRIDGE: virBufferEscapeString(buf, " <source bridge='%s'/>\n", def->data.bridge.brname); + if (def->data.bridge.script) + virBufferEscapeString(buf, " <script path='%s'/>\n", + def->data.bridge.script); break; case VIR_DOMAIN_NET_TYPE_SERVER: diff --git a/src/domain_conf.h b/src/domain_conf.h --- a/src/domain_conf.h +++ b/src/domain_conf.h @@ -153,6 +153,7 @@ struct _virDomainNetDef { } network; struct { char *brname; + char *script; } bridge; } data; char *ifname; diff --git a/src/xend_internal.c b/src/xend_internal.c --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -58,6 +58,12 @@ #define XEN_SCHED_CRED_NPARAM 2 #endif /* PROXY */ + +#ifdef __sun +#define DEFAULT_VIF_SCRIPT "vif-vnic" +#else +#define DEFAULT_VIF_SCRIPT "vif-bridge" +#endif /** * xend_connection_type: @@ -1775,15 +1781,22 @@ xenDaemonParseSxprNets(virConnectPtr con if (VIR_ALLOC(net) < 0) goto no_memory; - if ((tmp2 && strstr(tmp2, "bridge")) || tmp) { + if (tmp != NULL || (STREQ(tmp2, DEFAULT_VIF_SCRIPT))) { net->type = VIR_DOMAIN_NET_TYPE_BRIDGE; /* XXX virtual network reverse resolve */ if (tmp && !(net->data.bridge.brname = strdup(tmp))) goto no_memory; + if (tmp2 && + net->type == VIR_DOMAIN_NET_TYPE_BRIDGE && + !(net->data.bridge.script = strdup(tmp2))) + goto no_memory; } else { net->type = VIR_DOMAIN_NET_TYPE_ETHERNET; + if (tmp2 && + !(net->data.ethernet.script = strdup(tmp2))) + goto no_memory; } tmp = sexpr_node(node, "device/vif/vifname"); @@ -1820,11 +1833,6 @@ xenDaemonParseSxprNets(virConnectPtr con tmp = sexpr_node(node, "device/vif/ip"); if (tmp && !(net->data.ethernet.ipaddr = strdup(tmp))) - goto no_memory; - - if (tmp2 && - net->type == VIR_DOMAIN_NET_TYPE_ETHERNET && - !(net->data.ethernet.script = strdup(tmp2))) goto no_memory; if (model && @@ -5093,6 +5101,8 @@ xenDaemonFormatSxprNet(virConnectPtr con int xendConfigVersion, int isAttach) { + const char *script = DEFAULT_VIF_SCRIPT; + if (def->type != VIR_DOMAIN_NET_TYPE_BRIDGE && def->type != VIR_DOMAIN_NET_TYPE_NETWORK && def->type != VIR_DOMAIN_NET_TYPE_ETHERNET) { @@ -5114,7 +5124,10 @@ xenDaemonFormatSxprNet(virConnectPtr con switch (def->type) { case VIR_DOMAIN_NET_TYPE_BRIDGE: virBufferVSprintf(buf, "(bridge '%s')", def->data.bridge.brname); - virBufferAddLit(buf, "(script 'vif-bridge')"); + if (def->data.bridge.script) + script = def->data.bridge.script; + + virBufferVSprintf(buf, "(script '%s')", script); break; case VIR_DOMAIN_NET_TYPE_NETWORK: @@ -5137,7 +5150,7 @@ xenDaemonFormatSxprNet(virConnectPtr con return -1; } virBufferVSprintf(buf, "(bridge '%s')", bridge); - virBufferAddLit(buf, "(script 'vif-bridge')"); + virBufferVSprintf(buf, "(script '%s')", script); VIR_FREE(bridge); } break; diff --git a/src/xm_internal.c b/src/xm_internal.c --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -1072,14 +1072,19 @@ xenXMDomainConfigParse(virConnectPtr con net->mac[5] = rawmac[5]; } - if (bridge[0] || STREQ(script, "vif-bridge")) + if (bridge[0] || STREQ(script, "vif-bridge") || + STREQ(script, "vif-vnic")) { net->type = VIR_DOMAIN_NET_TYPE_BRIDGE; - else + } else { net->type = VIR_DOMAIN_NET_TYPE_ETHERNET; + } if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) { if (bridge[0] && !(net->data.bridge.brname = strdup(bridge))) + goto no_memory; + if (script[0] && + !(net->data.bridge.script = strdup(script))) goto no_memory; } else { if (script[0] &&

4 3

[libvirt] [PATCH] Avoid passing NULL to printf %s specifier
by john.levon＠sun.com 19 Jan '09

19 Jan '09

# HG changeset patch # User john.levon(a)sun.com # Date 1232032750 28800 # Node ID 0335828806b5b3855c8eeaee31446a7bd0c02974 # Parent e542223a4c52d978d4507de709f50885920a5b44 Avoid passing NULL to printf %s specifier This is non-portable. Signed-off-by: John Levon <john.levon(a)sun.com> diff --git a/src/internal.h b/src/internal.h --- a/src/internal.h +++ b/src/internal.h @@ -7,6 +7,7 @@ #include <errno.h> #include <limits.h> +#include <verify.h> #ifdef HAVE_SYS_SYSLIMITS_H #include <sys/syslimits.h> @@ -115,6 +116,13 @@ #define ATTRIBUTE_RETURN_CHECK #endif /* __GNUC__ */ +/* + * Use this when passing possibly-NULL strings to printf-a-likes. + */ +#define NULLSTR(s) \ + ((void)verify_true(sizeof *(s) == sizeof (char)), \ + (s) ? (s) : "(null)") + /** * TODO: * diff --git a/src/libvirt.c b/src/libvirt.c --- a/src/libvirt.c +++ b/src/libvirt.c @@ -864,10 +864,10 @@ do_open (const char *name, " port %d\n" " path %s\n", name, - ret->uri->scheme, ret->uri->opaque, - ret->uri->authority, ret->uri->server, - ret->uri->user, ret->uri->port, - ret->uri->path); + NULLSTR(ret->uri->scheme), NULLSTR(ret->uri->opaque), + NULLSTR(ret->uri->authority), NULLSTR(ret->uri->server), + NULLSTR(ret->uri->user), ret->uri->port, + NULLSTR(ret->uri->path)); } else { DEBUG0("no name, allowing driver auto-select"); } @@ -1056,7 +1056,7 @@ virConnectOpenAuth(const char *name, if (virInitialize() < 0) return NULL; - DEBUG("name=%s, auth=%p, flags=%d", name, auth, flags); + DEBUG("name=%s, auth=%p, flags=%d", NULLSTR(name), auth, flags); return do_open (name, auth, flags); }

4 3

[libvirt] Using vmchannel?
by Richard W.M. Jones 19 Jan '09

19 Jan '09

I was just looking at the new vmchannel feature in QEMU and was wondering if we could make this available in libvirt. Firstly, since there isn't much documentation, I should explain how vmchannel works: [1] You pass an extra parameter on the qemu command line: qemu [...] -vmchannel <port>:<dev> where <port> is the TCP port number (see below) and <dev> is a standard qemu device description. As an example: qemu [...] -vmchannel 600:unix:/some/path [2] A new character device appears in the host, eg. Unix domain socket called "/some/path". [3] In the host, a userspace program should open a socket, bind(2) it to /some/path and listen(2) and accept(2) on it. [4] In the guest, any process may connect(2) to TCP 10.0.2.4:600 (or whatever port was selected). Each connection in the guest causes the listener in the host to accept(2). [5] This is only designed for low-bandwidth, low-performance applications. [6] Multiple vmchannels are supported. [7] Host cannot initiate a connection. My plan to implement this would be to add a new network interface type to the domain XML: <interface type='vmchannel'> <source port='600'/> <target path='/some/path'/> </interface> Only Unix domain socket paths would be allowed on the host side, and the path would be expected to exist already with suitable permissions. Note that I think this would also allow guests to communicate with the libvirtd on the host (not by default, of course, but if users wanted to configure it that way), for example: <interface type='vmchannel'> <source port='600'/> <target path='/var/run/libvirt/libvirt-sock'/> </interface> One problem is that it is qemu/kvm-only. It is built on top of virtio, and virtio is meant to become a standard driver subsystem for all full virtualization systems. Thoughts? Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v

4 3

[libvirt] [PATCH 0/4] allow kvm/qemu to survive daemon restarts
by Guido Günther 19 Jan '09

19 Jan '09

Hi, attached is the current patch series for the above issue. The biggest change is in the 3rd patch that switches the qemu driver to use the monitor fd to detect daemon shutdown, therefore we can dup the vms stdout/err on the logfile right at startup. Cheers, -- Guido

1 0

[libvirt] kvm: save / restore
by Matthias Pfafferodt 17 Jan '09

17 Jan '09

Hallo, I use kvm-81 and libvirt 0.5.1. I can save a kvm donain but if I want to restore it I get the following error in the log file: unknown migration protocol: stdio I tried it using only kvm and got the same error. How can I save / restore a VM to / from a file? Kind regards Matthias Pfafferodt -- Matthias Pfafferodt - http://www.mapfa.de Matthias.Pfafferodt <at> mapfa.de

3 3

[libvirt] [PATCH] Fix vm define error with back compat console device
by Cole Robinson 17 Jan '09

17 Jan '09

If you define a domain with serial devs > 0 && parallel devs >= serial devs, libvirtd segfaults when trying to set up the back compat console device. We were using a previous loop counter where we shouldn't. The attached patch fixes this. Thanks, Cole

4 15

[libvirt] PATCH: Allow virtual networks to survive daemon restarts
by Daniel P. Berrange 17 Jan '09

17 Jan '09

Currently when we shutdown the virtual networks are all shutdown too. This is less than useful if we're letting guest VMs hang around post shutdown of libvirtd, because it means we're tearing their network connection out from under them. This patch fixes that allowing networks to survive restarts, and be re-detected next time around. When starting a virtual network we write the live config into /var/lib/libvirt/network/$NAME.xml This is because the bridge device name is potentially auto-generated and we need to keep track of that We change dnsmasq args to include an explicit pidfile location /var/run/libvirt/network/$NAME.pid and also tell it to put itself into the background - ie daemonize. This is because we want dnsmasq to survive the daemon. Now, when libvirtd starts up it - Looks for the live config, and if found loads it. - Calls a new method brHasBridge() to see if its desired bridge actually exists (and thus whether the network is running). If it exists,the network is marked active - If DHCP is configured, then reads the dnsmasq PIDfile, and sends kill($PID, 0) to check if the process is actually alive In addition I cleanup the network code to remove the configFile and autostartLink fields in virNetworkObjPtr, so it matches virDomaiObjPtr usage. With all this applied you can now restart the daemon, and virbr0 is left happily running. THis patch depends on the 25 threading patches I sent earlier, so you probably won't be able to apply it in isolation Daniel diff --git a/src/bridge.c b/src/bridge.c --- a/src/bridge.c +++ b/src/bridge.c @@ -163,6 +163,43 @@ int brAddBridge (brControl *ctl ATTRIBUT } #endif +#ifdef SIOCBRDELBR +int +brHasBridge(brControl *ctl, + const char *name) +{ + struct ifreq ifr; + int len; + + if (!ctl || !name) { + errno = EINVAL; + return -1; + } + + if ((len = strlen(name)) >= BR_IFNAME_MAXLEN) { + errno = EINVAL; + return -1; + } + + memset(&ifr, 0, sizeof(struct ifreq)); + + strncpy(ifr.ifr_name, name, len); + ifr.ifr_name[len] = '\0'; + + if (ioctl(ctl->fd, SIOCGIFMTU, &ifr)) + return -1; + + return 0; +} +#else +int +brHasBridge(brControl *ctl, + const char *name) +{ + return EINVAL; +} +#endif + /** * brDeleteBridge: * @ctl: bridge control pointer diff --git a/src/bridge.h b/src/bridge.h --- a/src/bridge.h +++ b/src/bridge.h @@ -50,6 +50,8 @@ int brAddBridge (brContr char **name); int brDeleteBridge (brControl *ctl, const char *name); +int brHasBridge (brControl *ctl, + const char *name); int brAddInterface (brControl *ctl, const char *bridge, @@ -58,6 +60,7 @@ int brDeleteInterface (brContr const char *bridge, const char *iface); + int brAddTap (brControl *ctl, const char *bridge, char **ifname, diff --git a/src/libvirt_bridge.syms b/src/libvirt_bridge.syms --- a/src/libvirt_bridge.syms +++ b/src/libvirt_bridge.syms @@ -9,6 +9,7 @@ brAddBridge; brAddInterface; brAddTap; brDeleteBridge; +brHasBridge; brInit; brSetEnableSTP; brSetForwardDelay; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -190,6 +190,7 @@ virFree; # network_conf.h virNetworkAssignDef; +virNetworkConfigFile; virNetworkDefFormat; virNetworkDefFree; virNetworkDefParseFile; @@ -202,6 +203,7 @@ virNetworkLoadAllConfigs; virNetworkObjListFree; virNetworkDefParseNode; virNetworkRemoveInactive; +virNetworkSaveConfigXML; virNetworkSaveConfig; virNetworkObjLock; virNetworkObjUnlock; diff --git a/src/network_conf.c b/src/network_conf.c --- a/src/network_conf.c +++ b/src/network_conf.c @@ -125,9 +125,6 @@ void virNetworkObjFree(virNetworkObjPtr virNetworkDefFree(net->def); virNetworkDefFree(net->newDef); - VIR_FREE(net->configFile); - VIR_FREE(net->autostartLink); - virMutexDestroy(&net->lock); VIR_FREE(net); @@ -641,31 +638,17 @@ char *virNetworkDefFormat(virConnectPtr return NULL; } -int virNetworkSaveConfig(virConnectPtr conn, - const char *configDir, - const char *autostartDir, - virNetworkObjPtr net) +int virNetworkSaveXML(virConnectPtr conn, + const char *configDir, + virNetworkDefPtr def, + const char *xml) { - char *xml; + char *configFile = NULL; int fd = -1, ret = -1; size_t towrite; int err; - if (!net->configFile && - virAsprintf(&net->configFile, "%s/%s.xml", - configDir, net->def->name) < 0) { - virNetworkReportError(conn, VIR_ERR_NO_MEMORY, NULL); - goto cleanup; - } - if (!net->autostartLink && - virAsprintf(&net->autostartLink, "%s/%s.xml", - autostartDir, net->def->name) < 0) { - virNetworkReportError(conn, VIR_ERR_NO_MEMORY, NULL); - goto cleanup; - } - - if (!(xml = virNetworkDefFormat(conn, - net->newDef ? net->newDef : net->def))) + if ((configFile = virNetworkConfigFile(conn, configDir, def->name)) == NULL) goto cleanup; if ((err = virFileMakePath(configDir))) { @@ -675,19 +658,12 @@ int virNetworkSaveConfig(virConnectPtr c goto cleanup; } - if ((err = virFileMakePath(autostartDir))) { - virReportSystemError(conn, err, - _("cannot create autostart directory '%s'"), - autostartDir); - goto cleanup; - } - - if ((fd = open(net->configFile, + if ((fd = open(configFile, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR )) < 0) { virReportSystemError(conn, errno, _("cannot create config file '%s'"), - net->configFile); + configFile); goto cleanup; } @@ -695,48 +671,64 @@ int virNetworkSaveConfig(virConnectPtr c if (safewrite(fd, xml, towrite) < 0) { virReportSystemError(conn, errno, _("cannot write config file '%s'"), - net->configFile); + configFile); goto cleanup; } if (close(fd) < 0) { virReportSystemError(conn, errno, _("cannot save config file '%s'"), - net->configFile); + configFile); goto cleanup; } ret = 0; cleanup: - VIR_FREE(xml); if (fd != -1) close(fd); + VIR_FREE(configFile); + return ret; } +int virNetworkSaveConfig(virConnectPtr conn, + const char *configDir, + virNetworkDefPtr def) +{ + int ret = -1; + char *xml; + + if (!(xml = virNetworkDefFormat(conn, + def))) + goto cleanup; + + if (virNetworkSaveXML(conn, configDir, def, xml)) + goto cleanup; + + ret = 0; +cleanup: + VIR_FREE(xml); + return ret; +} + + virNetworkObjPtr virNetworkLoadConfig(virConnectPtr conn, virNetworkObjListPtr nets, const char *configDir, const char *autostartDir, - const char *file) + const char *name) { char *configFile = NULL, *autostartLink = NULL; virNetworkDefPtr def = NULL; virNetworkObjPtr net; int autostart; - if (virAsprintf(&configFile, "%s/%s", - configDir, file) < 0) { - virNetworkReportError(conn, VIR_ERR_NO_MEMORY, NULL); + if ((configFile = virNetworkConfigFile(conn, configDir, name)) == NULL) goto error; - } - if (virAsprintf(&autostartLink, "%s/%s", - autostartDir, file) < 0) { - virNetworkReportError(conn, VIR_ERR_NO_MEMORY, NULL); + if ((autostartLink = virNetworkConfigFile(conn, autostartDir, name)) == NULL) goto error; - } if ((autostart = virFileLinkPointsTo(autostartLink, configFile)) < 0) goto error; @@ -744,7 +736,7 @@ virNetworkObjPtr virNetworkLoadConfig(vi if (!(def = virNetworkDefParseFile(conn, configFile))) goto error; - if (!virFileMatchesNameSuffix(file, def->name, ".xml")) { + if (!STREQ(name, def->name)) { virNetworkReportError(conn, VIR_ERR_INTERNAL_ERROR, _("Network config filename '%s'" " does not match network name '%s'"), @@ -755,10 +747,11 @@ virNetworkObjPtr virNetworkLoadConfig(vi if (!(net = virNetworkAssignDef(conn, nets, def))) goto error; - net->configFile = configFile; - net->autostartLink = autostartLink; net->autostart = autostart; + VIR_FREE(configFile); + VIR_FREE(autostartLink); + return net; error: @@ -791,7 +784,7 @@ int virNetworkLoadAllConfigs(virConnectP if (entry->d_name[0] == '.') continue; - if (!virFileHasSuffix(entry->d_name, ".xml")) + if (!virFileStripSuffix(entry->d_name, ".xml")) continue; /* NB: ignoring errors, so one malformed config doesn't @@ -811,27 +804,51 @@ int virNetworkLoadAllConfigs(virConnectP } int virNetworkDeleteConfig(virConnectPtr conn, + const char *configDir, + const char *autostartDir, virNetworkObjPtr net) { - if (!net->configFile || !net->autostartLink) { - virNetworkReportError(conn, VIR_ERR_INTERNAL_ERROR, - _("no config file for %s"), net->def->name); - return -1; - } + char *configFile = NULL; + char *autostartLink = NULL; + + if ((configFile = virNetworkConfigFile(conn, configDir, net->def->name)) == NULL) + goto error; + if ((autostartLink = virNetworkConfigFile(conn, autostartDir, net->def->name)) == NULL) + goto error; /* Not fatal if this doesn't work */ - unlink(net->autostartLink); + unlink(autostartLink); - if (unlink(net->configFile) < 0) { + if (unlink(configFile) < 0) { virReportSystemError(conn, errno, _("cannot remove config file '%s'"), - net->configFile); - return -1; + configFile); + goto error; } return 0; + +error: + VIR_FREE(configFile); + VIR_FREE(autostartLink); + return -1; } +char *virNetworkConfigFile(virConnectPtr conn, + const char *dir, + const char *name) +{ + char *ret = NULL; + + if (virAsprintf(&ret, "%s/%s.xml", dir, name) < 0) { + virNetworkReportError(conn, VIR_ERR_NO_MEMORY, NULL); + return NULL; + } + + return ret; +} + + void virNetworkObjLock(virNetworkObjPtr obj) { virMutexLock(&obj->lock); diff --git a/src/network_conf.h b/src/network_conf.h --- a/src/network_conf.h +++ b/src/network_conf.h @@ -90,9 +90,6 @@ struct _virNetworkObj { unsigned int autostart : 1; unsigned int persistent : 1; - char *configFile; /* Persistent config file path */ - char *autostartLink; /* Symlink path for autostart */ - virNetworkDefPtr def; /* The current definition */ virNetworkDefPtr newDef; /* New definition to activate at shutdown */ }; @@ -139,10 +136,14 @@ char *virNetworkDefFormat(virConnectPtr const virNetworkDefPtr def); +int virNetworkSaveXML(virConnectPtr conn, + const char *configDir, + virNetworkDefPtr def, + const char *xml); + int virNetworkSaveConfig(virConnectPtr conn, const char *configDir, - const char *autostartDir, - virNetworkObjPtr net); + virNetworkDefPtr def); virNetworkObjPtr virNetworkLoadConfig(virConnectPtr conn, virNetworkObjListPtr nets, @@ -156,8 +157,15 @@ int virNetworkLoadAllConfigs(virConnectP const char *autostartDir); int virNetworkDeleteConfig(virConnectPtr conn, + const char *configDir, + const char *autostartDir, virNetworkObjPtr net); +char *virNetworkConfigFile(virConnectPtr conn, + const char *dir, + const char *name); + + void virNetworkObjLock(virNetworkObjPtr obj); void virNetworkObjUnlock(virNetworkObjPtr obj); diff --git a/src/network_driver.c b/src/network_driver.c --- a/src/network_driver.c +++ b/src/network_driver.c @@ -57,6 +57,8 @@ #include "iptables.h" #include "bridge.h" +#define NETWORK_PID_DIR LOCAL_STATE_DIR "/run/libvirt/network" +#define NETWORK_LIB_DIR LOCAL_STATE_DIR "/lib/libvirt/network" #define VIR_FROM_THIS VIR_FROM_NETWORK @@ -106,6 +108,64 @@ static struct network_driver *driverStat static void +networkFindActiveConfigs(struct network_driver *driver) { + unsigned int i; + + for (i = 0 ; i < driver->networks.count ; i++) { + virNetworkObjPtr obj = driver->networks.objs[i]; + virNetworkDefPtr tmp; + char *config; + + virNetworkObjLock(obj); + + if ((config = virNetworkConfigFile(NULL, + NETWORK_LIB_DIR, + obj->def->name)) == NULL) { + virNetworkObjUnlock(obj); + continue; + } + + if (access(config, R_OK) < 0) { + VIR_FREE(config); + virNetworkObjUnlock(obj); + continue; + } + + /* Try and load the live config */ + tmp = virNetworkDefParseFile(NULL,config); + VIR_FREE(config); + if (tmp) { + obj->newDef = obj->def; + obj->def = tmp; + } + + /* If bridge exists, then mark it active */ + if (obj->def->bridge && + brHasBridge(driver->brctl, obj->def->bridge) == 0) { + obj->active = 1; + + /* Finally try and read dnsmasq pid if any DHCP ranges are set */ + if (obj->def->nranges && + virFileReadPid(NETWORK_PID_DIR, obj->def->name, + &obj->dnsmasqPid) == 0) { + + /* Check its still alive */ + if (kill(obj->dnsmasqPid, 0) != 0) + obj->dnsmasqPid = -1; + + /* XXX ideally we'd check this was actually + * the dnsmasq process, not a stale pid file + * with someone else's process. But how ? + */ + } + } + + virNetworkObjUnlock(obj); + } +} + + +static void networkAutostartConfigs(struct network_driver *driver) { unsigned int i; @@ -132,6 +192,7 @@ static int networkStartup(void) { uid_t uid = geteuid(); char *base = NULL; + int err; if (VIR_ALLOC(driverState) < 0) goto error; @@ -181,12 +242,26 @@ networkStartup(void) { VIR_FREE(base); + if ((err = brInit(&driverState->brctl))) { + virReportSystemError(NULL, err, "%s", + _("cannot initialize bridge support")); + goto error; + } + + if (!(driverState->iptables = iptablesContextNew())) { + networkReportError(NULL, NULL, NULL, VIR_ERR_NO_MEMORY, + "%s", _("failed to allocate space for IP tables support")); + goto error; + } + + if (virNetworkLoadAllConfigs(NULL, &driverState->networks, driverState->networkConfigDir, driverState->networkAutostartDir) < 0) goto error; + networkFindActiveConfigs(driverState); networkAutostartConfigs(driverState); networkDriverUnlock(driverState); @@ -269,23 +344,11 @@ networkActive(void) { */ static int networkShutdown(void) { - unsigned int i; - if (!driverState) return -1; networkDriverLock(driverState); - /* shutdown active networks */ - for (i = 0 ; i < driverState->networks.count ; i++) { - virNetworkObjPtr net = driverState->networks.objs[i]; - virNetworkObjLock(net); - if (virNetworkIsActive(net)) - networkShutdownNetworkDaemon(NULL, driverState, - driverState->networks.objs[i]); - virNetworkObjUnlock(net); - } - /* free inactive networks */ virNetworkObjListFree(&driverState->networks); @@ -309,23 +372,23 @@ networkShutdown(void) { static int networkBuildDnsmasqArgv(virConnectPtr conn, - virNetworkObjPtr network, - const char ***argv) { + virNetworkObjPtr network, + const char *pidfile, + const char ***argv) { int i, len, r; - char buf[PATH_MAX]; + char *pidfileArg; + char buf[1024]; len = 1 + /* dnsmasq */ - 1 + /* --keep-in-foreground */ 1 + /* --strict-order */ 1 + /* --bind-interfaces */ (network->def->domain?2:0) + /* --domain name */ - 2 + /* --pid-file "" */ + 2 + /* --pid-file /var/run/libvirt/network/$NAME.pid */ 2 + /* --conf-file "" */ /*2 + *//* --interface virbr0 */ 2 + /* --except-interface lo */ 2 + /* --listen-address 10.0.0.1 */ - 1 + /* --dhcp-leasefile=path */ (2 * network->def->nranges) + /* --dhcp-range 10.0.0.2,10.0.0.254 */ /* --dhcp-host 01:23:45:67:89:0a,hostname,10.0.0.3 */ (2 * network->def->nhosts) + @@ -339,11 +402,13 @@ networkBuildDnsmasqArgv(virConnectPtr co goto no_memory; \ } while (0) +#define APPEND_ARG_LIT(v, n, s) \ + (v)[(n)] = s + i = 0; APPEND_ARG(*argv, i++, DNSMASQ); - APPEND_ARG(*argv, i++, "--keep-in-foreground"); /* * Needed to ensure dnsmasq uses same algorithm for processing * multiple namedriver entries in /etc/resolv.conf as GLibC. @@ -356,10 +421,11 @@ networkBuildDnsmasqArgv(virConnectPtr co APPEND_ARG(*argv, i++, network->def->domain); } - APPEND_ARG(*argv, i++, "--pid-file"); - APPEND_ARG(*argv, i++, ""); + if (virAsprintf(&pidfileArg, "--pid-file=%s", pidfile) < 0) + goto no_memory; + APPEND_ARG_LIT(*argv, i++, pidfileArg); - APPEND_ARG(*argv, i++, "--conf-file"); + APPEND_ARG(*argv, i++, "--conf-file="); APPEND_ARG(*argv, i++, ""); /* @@ -377,15 +443,6 @@ networkBuildDnsmasqArgv(virConnectPtr co APPEND_ARG(*argv, i++, "--except-interface"); APPEND_ARG(*argv, i++, "lo"); - /* - * NB, dnsmasq command line arg bug means we need to - * use a single arg '--dhcp-leasefile=path' rather than - * two separate args in '--dhcp-leasefile path' style - */ - snprintf(buf, sizeof(buf), "--dhcp-leasefile=%s/lib/libvirt/dhcp-%s.leases", - LOCAL_STATE_DIR, network->def->name); - APPEND_ARG(*argv, i++, buf); - for (r = 0 ; r < network->def->nranges ; r++) { snprintf(buf, sizeof(buf), "%s,%s", network->def->ranges[r].start, @@ -434,7 +491,10 @@ dhcpStartDhcpDaemon(virConnectPtr conn, virNetworkObjPtr network) { const char **argv; - int ret, i; + char *pidfile; + int ret = -1, i, err; + + network->dnsmasqPid = -1; if (network->def->ipAddress == NULL) { networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, @@ -442,13 +502,49 @@ dhcpStartDhcpDaemon(virConnectPtr conn, return -1; } + if ((err = virFileMakePath(NETWORK_PID_DIR)) < 0) { + virReportSystemError(conn, err, + _("cannot create directory %s"), + NETWORK_PID_DIR); + return -1; + } + if ((err = virFileMakePath(NETWORK_LIB_DIR)) < 0) { + virReportSystemError(conn, err, + _("cannot create directory %s"), + NETWORK_LIB_DIR); + return -1; + } + + if (!(pidfile = virFilePid(NETWORK_PID_DIR, network->def->name))) { + virReportOOMError(conn); + return -1; + } + argv = NULL; - if (networkBuildDnsmasqArgv(conn, network, &argv) < 0) + if (networkBuildDnsmasqArgv(conn, network, pidfile, &argv) < 0) { + VIR_FREE(pidfile); return -1; + } - ret = virExec(conn, argv, NULL, NULL, - &network->dnsmasqPid, -1, NULL, NULL, VIR_EXEC_NONBLOCK); + if (virRun(conn, argv, NULL) < 0) + goto cleanup; + /* + * There really is no race here - when dnsmasq daemonizes, + * its leader process stays around until its child has + * actually written its pidfile. So by time virRun exits + * it has waitpid'd and guarenteed the proess has started + * and writtena pid + */ + + if (virFileReadPid(NETWORK_PID_DIR, network->def->name, + &network->dnsmasqPid) < 0) + goto cleanup; + + ret = 0; + +cleanup: + VIR_FREE(pidfile); for (i = 0; argv[i]; i++) VIR_FREE(argv[i]); VIR_FREE(argv); @@ -554,13 +650,6 @@ networkAddIptablesRules(virConnectPtr co virNetworkObjPtr network) { int err; - if (!driver->iptables && !(driver->iptables = iptablesContextNew())) { - networkReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, - "%s", _("failed to allocate space for IP tables support")); - return 0; - } - - /* allow DHCP requests through to dnsmasq */ if ((err = iptablesAddTcpInput(driver->iptables, network->def->bridge, 67))) { virReportSystemError(conn, err, @@ -716,12 +805,6 @@ static int networkStartNetworkDaemon(vir return -1; } - if (!driver->brctl && (err = brInit(&driver->brctl))) { - virReportSystemError(conn, err, "%s", - _("cannot initialize bridge support")); - return -1; - } - if ((err = brAddBridge(driver->brctl, &network->def->bridge))) { virReportSystemError(conn, err, _("cannot create bridge '%s'"), @@ -729,7 +812,6 @@ static int networkStartNetworkDaemon(vir return -1; } - if (brSetForwardDelay(driver->brctl, network->def->bridge, network->def->delay) < 0) goto err_delbr; @@ -774,10 +856,22 @@ static int networkStartNetworkDaemon(vir dhcpStartDhcpDaemon(conn, network) < 0) goto err_delbr2; + + /* Persist the live configuration now we have bridge info */ + if (virNetworkSaveConfig(conn, NETWORK_LIB_DIR, network->def) < 0) { + goto err_kill; + } + network->active = 1; return 0; + err_kill: + if (network->dnsmasqPid > 0) { + kill(network->dnsmasqPid, SIGTERM); + network->dnsmasqPid = -1; + } + err_delbr2: networkRemoveIptablesRules(driver, network); @@ -798,16 +892,24 @@ static int networkStartNetworkDaemon(vir } -static int networkShutdownNetworkDaemon(virConnectPtr conn ATTRIBUTE_UNUSED, - struct network_driver *driver, - virNetworkObjPtr network) { +static int networkShutdownNetworkDaemon(virConnectPtr conn, + struct network_driver *driver, + virNetworkObjPtr network) { int err; + char *configFile; networkLog(NETWORK_INFO, _("Shutting down network '%s'\n"), network->def->name); if (!virNetworkIsActive(network)) return 0; + configFile = virNetworkConfigFile(conn, NETWORK_LIB_DIR, network->def->name); + if (!configFile) + return -1; + + unlink(configFile); + VIR_FREE(configFile); + if (network->dnsmasqPid > 0) kill(network->dnsmasqPid, SIGTERM); @@ -824,13 +926,10 @@ static int networkShutdownNetworkDaemon( network->def->bridge, strerror(err)); } + /* See if its still alive and really really kill it */ if (network->dnsmasqPid > 0 && - waitpid(network->dnsmasqPid, NULL, WNOHANG) != network->dnsmasqPid) { + (kill(network->dnsmasqPid, 0) == 0)) kill(network->dnsmasqPid, SIGKILL); - if (waitpid(network->dnsmasqPid, NULL, 0) != network->dnsmasqPid) - networkLog(NETWORK_WARN, - "%s", _("Got unexpected pid for dnsmasq\n")); - } network->dnsmasqPid = -1; network->active = 0; @@ -1048,8 +1147,7 @@ static virNetworkPtr networkDefine(virCo if (virNetworkSaveConfig(conn, driver->networkConfigDir, - driver->networkAutostartDir, - network) < 0) { + network->newDef ? network->newDef : network->def) < 0) { virNetworkRemoveInactive(&driver->networks, network); network = NULL; @@ -1086,7 +1184,10 @@ static int networkUndefine(virNetworkPtr goto cleanup; } - if (virNetworkDeleteConfig(net->conn, network) < 0) + if (virNetworkDeleteConfig(net->conn, + driver->networkConfigDir, + driver->networkAutostartDir, + network) < 0) goto cleanup; virNetworkRemoveInactive(&driver->networks, @@ -1140,7 +1241,7 @@ static int networkDestroy(virNetworkPtr } ret = networkShutdownNetworkDaemon(net->conn, driver, network); - if (!network->configFile) { + if (!network->persistent) { virNetworkRemoveInactive(&driver->networks, network); network = NULL; @@ -1233,9 +1334,10 @@ cleanup: } static int networkSetAutostart(virNetworkPtr net, - int autostart) { + int autostart) { struct network_driver *driver = net->conn->networkPrivateData; virNetworkObjPtr network; + char *configFile = NULL, *autostartLink = NULL; int ret = -1; networkDriverLock(driver); @@ -1251,6 +1353,11 @@ static int networkSetAutostart(virNetwor autostart = (autostart != 0); if (network->autostart != autostart) { + if ((configFile = virNetworkConfigFile(net->conn, driver->networkConfigDir, network->def->name)) == NULL) + goto cleanup; + if ((autostartLink = virNetworkConfigFile(net->conn, driver->networkAutostartDir, network->def->name)) == NULL) + goto cleanup; + if (autostart) { int err; @@ -1261,17 +1368,17 @@ static int networkSetAutostart(virNetwor goto cleanup; } - if (symlink(network->configFile, network->autostartLink) < 0) { + if (symlink(configFile, autostartLink) < 0) { virReportSystemError(net->conn, errno, _("Failed to create symlink '%s' to '%s'"), - network->autostartLink, network->configFile); + autostartLink, configFile); goto cleanup; } } else { - if (unlink(network->autostartLink) < 0 && errno != ENOENT && errno != ENOTDIR) { + if (unlink(autostartLink) < 0 && errno != ENOENT && errno != ENOTDIR) { virReportSystemError(net->conn, errno, _("Failed to delete symlink '%s'"), - network->autostartLink); + autostartLink); goto cleanup; } } @@ -1281,6 +1388,8 @@ static int networkSetAutostart(virNetwor ret = 0; cleanup: + VIR_FREE(configFile); + VIR_FREE(autostartLink); if (network) virNetworkObjUnlock(network); return ret; -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

3 4

[libvirt] sharing multiple identical USB devices?
by Dr. Michael J. Chudobiak 16 Jan '09

16 Jan '09

Hi, I have Windows XP running on Fedora 10. I can successfully share a single USB-to-RS232 converter by inserting this in my xml file: <hostdev mode='subsystem' type='usb'> <source> <vendor id='0x0403'/> <product id='0x6001'/> </source> </hostdev> However, I can't seem to get two of these to work at the same time. I want to do: <hostdev mode='subsystem' type='usb'> <source> <vendor id='0x0403'/> <product id='0x6001'/> <address bus='0x03' device='0x02'/> </source> </hostdev> <hostdev mode='subsystem' type='usb'> <source> <vendor id='0x0403'/> <product id='0x6001'/> <address bus='0x03' device='0x03'/> </source> </hostdev> using the "address" stanza to identify the devices uniquely, but it just doesn't work. Is this a bug, or am I doing something wrong? With the two device setup, Windows still only assigns one serial port to one of the devices, and none to the other. - Mike

3 3

[libvirt] Fine grained Access Control in libVirt
by Konrad Eriksson1 16 Jan '09

16 Jan '09

Hi, After some background research it doesn't look like anyone have taken on the task yet to add fine-grained access control to libVirt (only option right now is R/W or RO mode). Desired is an addition to libVirt that enables access control on individual actions and data that can be accessed through the library API. This could take the form of an AC-module that, based on the identity of the caller, checks each call and grants/denies access to carry out the action (could also take parameters in account) and optionally filter the return data. The AC-module could then interface different backend AC solutions (SELinux, RBAC, ...) or alternatively implement an internal scheme. When looking at the libvirt core and driver framework it seems promising to inject these kind of call-out hooks either in libvirt.c or between libvirt.c and the underlying drivers, by doing this AC will be enforced independent of if a local or remote call is done to libVirt. I propose an approach to create an AC-module that conforms to the driver API in libVirt and to inject it in the call-path between libvirt.c and the driver(s) to enable the AC-module to inspect the call before sending it to the real driver. Normal call path: user -> libvirt.c -> driver_x AC-module injected in call path: user -> libvirt.c -> AC-module -> driver_x By doing this it can be loaded/unloaded in run-time and also selectable what driver paths it should enforce (hypervisor(aka. driver), storage, network...). The AC-module can also be made in different flavors for different AC backend (SELinux, RBAC, internal, ...) solutions and the appropriate AC-module could be loaded without needing re-compiling. This approach would also leave a very small footprint in existing libvirt code (only need to inject AC-module driver after normal drivers has been loaded) Feel free to comment and to come with improvement ideas. Freundliche Grüsse / Best regards Konrad Eriksson Trusted Computing / Security & Assurance IBM Zurich Research Laboratory Saeumerstrasse 4 8803 Rueschlikon Switzerland

4 5