- Devel - libvirt List Archives

[libvirt] [PATCH 0/2] snapshot: add force for risky reverts
by Eric Blake 05 Oct '11

05 Oct '11

I first documented the need for force back in my RFC: https://www.redhat.com/archives/libvir-list/2011-August/msg00361.html but only now got around to implementing it. At the moment, I'm posting the code for early review. I'm still in the process of testing out multiple scenarios, and will send followup mail detailing the actual steps I took using virsh to cover the multiple scenarios. Eric Blake (2): snapshot: add REVERT_FORCE to API snapshot: enforce REVERT_FORCE on qemu include/libvirt/libvirt.h.in | 1 + include/libvirt/virterror.h | 2 + src/libvirt.c | 22 +++++++++++++++++++ src/qemu/qemu_driver.c | 47 +++++++++++++++++++++++++++++++++--------- src/util/virterror.c | 6 +++++ tools/virsh.c | 19 ++++++++++++++++- tools/virsh.pod | 17 +++++++++++++++ 7 files changed, 103 insertions(+), 11 deletions(-) -- 1.7.4.4

2 11

[libvirt] [PATCH] qemu: Don't fail virDomainGetInfo if we can't update balloon info
by Jiri Denemark 05 Oct '11

05 Oct '11

Qemu driver tries to update balloon data in virDomainGetInfo and if it can't do so because there is another monitor job running, it just reports what's known in domain def. However, if there was no job running but getting the data from qemu fails, we would fail the whole API. This doesn't make sense. Let's make the failure nonfatal. --- src/qemu/qemu_driver.c | 13 +++++++++---- 1 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index dc34e1d..276dc06 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2071,37 +2071,42 @@ static int qemudDomainGetInfo(virDomainPtr dom, } else if (qemuDomainJobAllowed(priv, QEMU_JOB_QUERY)) { if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) goto cleanup; if (!virDomainObjIsActive(vm)) err = 0; else { qemuDomainObjEnterMonitor(driver, vm); err = qemuMonitorGetBalloonInfo(priv->mon, &balloon); qemuDomainObjExitMonitor(driver, vm); } if (qemuDomainObjEndJob(driver, vm) == 0) { vm = NULL; goto cleanup; } - if (err < 0) - goto cleanup; - if (err == 0) + if (err < 0) { + /* We couldn't get current memory allocation but that's not + * a show stopper; we wouldn't get it if there was a job + * active either + */ + info->memory = vm->def->mem.cur_balloon; + } else if (err == 0) { /* Balloon not supported, so maxmem is always the allocation */ info->memory = vm->def->mem.max_balloon; - else + } else { info->memory = balloon; + } } else { info->memory = vm->def->mem.cur_balloon; } } else { info->memory = vm->def->mem.cur_balloon; } info->nrVirtCpu = vm->def->vcpus; ret = 0; cleanup: if (vm) virDomainObjUnlock(vm); return ret; } -- 1.7.7

2 2

[libvirt] [RFC] security_dac: don't chown iso file
by Serge E. Hallyn 05 Oct '11

05 Oct '11

isos are read-only, so libvirt doesn't need to chown them. In one of our testing setups, libvirt uses mirrorred isos. Since libvirt chowns the files, (and especially does not chown them back) the mirror refuses to update the iso. This patch prevents libvirt from chowning files. Does this seem reasonable? Signed-off-by: Serge Hallyn <serge.hallyn(a)canonical.com> --- src/security/security_dac.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index af02236..e7db324 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -555,6 +555,8 @@ virSecurityDACSetSecurityAllLabel(virSecurityManagerPtr mgr, /* XXX fixme - we need to recursively label the entire tree :-( */ if (vm->def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_DIR) continue; + if (vm->def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_CDROM) + continue; if (virSecurityDACSetSecurityImageLabel(mgr, vm, vm->def->disks[i]) < 0) -- 1.7.5.4

4 5

[libvirt] [PATCH] qemu: Fix migration with dname
by Jiri Denemark 05 Oct '11

05 Oct '11

Destination libvirtd remembers the original name in the prepare phase and clears it in the finish phase. The original name is used when comparing domain name in migration cookie. --- Notes: Originally, I wanted to transfer the new name in migration cookie but that appeared to be much more complicated and it would require adding new Confirm API since the current version does not have 'dname' parameter. src/qemu/qemu_domain.c | 1 + src/qemu/qemu_domain.h | 1 + src/qemu/qemu_migration.c | 19 ++++++++++++++++--- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d3ad192..65f721a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -231,6 +231,7 @@ static void qemuDomainObjPrivateFree(void *data) qemuDomainObjFreeJob(priv); VIR_FREE(priv->vcpupids); VIR_FREE(priv->lockState); + VIR_FREE(priv->origname); /* This should never be non-NULL if we get here, but just in case... */ if (priv->mon) { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index cdf1375..d9f323c 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -126,6 +126,7 @@ struct _qemuDomainObjPrivate { int jobs_queued; unsigned long migMaxBandwidth; + char *origname; }; struct qemuDomainWatchdogEvent diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 1122dab..4516231 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -254,12 +254,18 @@ error: static qemuMigrationCookiePtr qemuMigrationCookieNew(virDomainObjPtr dom) { + qemuDomainObjPrivatePtr priv = dom->privateData; qemuMigrationCookiePtr mig = NULL; + const char *name; if (VIR_ALLOC(mig) < 0) goto no_memory; - if (!(mig->name = strdup(dom->def->name))) + if (priv->origname) + name = priv->origname; + else + name = dom->def->name; + if (!(mig->name = strdup(name))) goto no_memory; memcpy(mig->uuid, dom->def->uuid, VIR_UUID_BUFLEN); @@ -1064,6 +1070,7 @@ qemuMigrationPrepareAny(struct qemud_driver *driver, unsigned long long now; qemuMigrationCookiePtr mig = NULL; bool tunnel = !!st; + char *origname = NULL; if (virTimeMs(&now) < 0) return -1; @@ -1078,7 +1085,7 @@ qemuMigrationPrepareAny(struct qemud_driver *driver, /* Target domain name, maybe renamed. */ if (dname) { - VIR_FREE(def->name); + origname = def->name; def->name = strdup(dname); if (def->name == NULL) goto cleanup; @@ -1095,6 +1102,8 @@ qemuMigrationPrepareAny(struct qemud_driver *driver, } def = NULL; priv = vm->privateData; + priv->origname = origname; + origname = NULL; if (!(mig = qemuMigrationEatCookie(driver, vm, cookiein, cookieinlen, QEMU_MIGRATION_COOKIE_LOCKSTATE))) @@ -1175,6 +1184,7 @@ qemuMigrationPrepareAny(struct qemud_driver *driver, ret = 0; cleanup: + VIR_FREE(origname); virDomainDefFree(def); VIR_FORCE_CLOSE(dataFD[0]); VIR_FORCE_CLOSE(dataFD[1]); @@ -2542,6 +2552,7 @@ qemuMigrationFinish(struct qemud_driver *driver, qemuMigrationCookiePtr mig = NULL; virErrorPtr orig_err = NULL; int cookie_flags = 0; + qemuDomainObjPrivatePtr priv = vm->privateData; VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, " "cookieout=%p, cookieoutlen=%p, flags=%lx, retcode=%d", @@ -2695,8 +2706,10 @@ endjob: } cleanup: - if (vm) + if (vm) { + VIR_FREE(priv->origname); virDomainObjUnlock(vm); + } if (event) qemuDomainEventQueue(driver, event); qemuMigrationCookieFree(mig); -- 1.7.7

3 6

[libvirt] "Connection does not support host device enumeration"
by Kaushal Shriyan 05 Oct '11

05 Oct '11

Hi, I have "20:04.0 Network controller: Sangoma Technologies Corp. A104d QUAD T1/E1 AFT card" on Host OS, Its not visible on guest OS using linux KVM application. I did open the window for guest from virt-manager on my Ubuntu Linux Desktop 11.04, shut down the guest, then select the "Details" view from the menu on that window, and click "Add Hardware" at the bottom, select "PCI Host Device" in the selections on the left, and find your device in the list of host devices on the right. Then click "Finish". Finally, start your guest up again, and the device should appear. I get "Connection does not support host device enumeration" Any clue ? libvirt-0.8.2-22.el5 version running on CentOS Linux Server version 5.6 Please let me know if you need any additional information Regards, Kaushal

3 10

[libvirt] [RFC PATCH] snapshot: better virsh handling of missing current, parent
by Eric Blake 04 Oct '11

04 Oct '11

Previously, virsh 'snapshot-parent' and 'snapshot-current' were completely silent in the case where the code conclusively proved there was no parent or current snapshot, but differed in exit status; this silence caused some confusion on whether the commands worked. Furthermore, commit d1be48f introduced a regression where snapshot-parent would leak output about an unknown function, but only on the first attempt, when talking to an older server that lacks virDomainSnapshotGetParent. This changes things to consistenly report an error message and exit with status 1 when no snapshot exists, and to avoid leaking unknown function warnings when using fallbacks. * tools/virsh.c (vshGetSnapshotParent): Alter signature, to distinguish between real error and missing parent. Don't pollute last_error on success. (cmdSnapshotParent): Adjust caller. Always output message on failure. (vshGetSnapshotParent): Adjust caller. (cmdSnapshotCurrent): Always output message on failure. --- This patch is an RFC because of backwards-compatibility concerns: Currently, snapshot-current outputs nothing but has status 0 if there is no current snapshot; but snapshot-parent outputs nothing but has status 1 if there is no parent snapshot. Either way, we have an inconsistency that needs to be fixed, and gaining consistency means breaking backwards compatibility with at least one command. Approach 1 (this patch): change snapshot-parent and snapshot-current to always output something, whether to stdout on success or to stderr on failure, with lack of a snapshot being considered a failure (where snapshot-current used to treat it as success). Approach 2 (not written) since snapshot-current existed much longer, makesnapshot-parent consistent by giving status 0 when it is silent. Preferences? (I guess mine is approach 1, by evidence of this patch). tools/virsh.c | 54 +++++++++++++++++++++++++++++++++++++++--------------- 1 files changed, 39 insertions(+), 15 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 1909dce..179cda5 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -12945,6 +12945,7 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd) char *xml = NULL; const char *snapshotname = NULL; unsigned int flags = 0; + const char *domname; if (vshCommandOptBool(cmd, "security-info")) flags |= VIR_DOMAIN_XML_SECURE; @@ -12952,7 +12953,7 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd) if (!vshConnectionUsability(ctl, ctl->conn)) goto cleanup; - dom = vshCommandOptDomain(ctl, cmd, NULL); + dom = vshCommandOptDomain(ctl, cmd, &domname); if (dom == NULL) goto cleanup; @@ -12986,9 +12987,12 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd) } current = virDomainHasCurrentSnapshot(dom, 0); - if (current < 0) + if (current < 0) { + goto cleanup; + } else if (!current) { + vshError(ctl, _("domain '%s' has no current snapshot"), domname); goto cleanup; - else if (current) { + } else { const char *name = NULL; if (!(snapshot = virDomainSnapshotCurrent(dom, 0))) @@ -13010,6 +13014,8 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd) ret = true; cleanup: + if (!ret) + virshReportError(ctl); VIR_FREE(xml); if (snapshot) virDomainSnapshotFree(snapshot); @@ -13020,26 +13026,33 @@ cleanup: } /* Helper function to get the name of a snapshot's parent. Caller - * must free the result. */ -static char * -vshGetSnapshotParent(vshControl *ctl, virDomainSnapshotPtr snapshot) + * must free the result. Returns 0 on success (including when it was + * proven no parent exists), and -1 on failure with error reported + * (such as no snapshot support or domain deleted in meantime). */ +static int +vshGetSnapshotParent(vshControl *ctl, virDomainSnapshotPtr snapshot, + char **parent_name) { virDomainSnapshotPtr parent = NULL; char *xml = NULL; xmlDocPtr xmldoc = NULL; xmlXPathContextPtr ctxt = NULL; - char *parent_name = NULL; + int ret = -1; + + *parent_name = NULL; /* Try new API, since it is faster. */ if (!ctl->useSnapshotGetXML) { parent = virDomainSnapshotGetParent(snapshot, 0); if (parent) { - /* API works, and virDomainSnapshotGetName will succeed */ - parent_name = vshStrdup(ctl, virDomainSnapshotGetName(parent)); + /* API works, and virDomainSnapshotGetName will be accurate */ + *parent_name = vshStrdup(ctl, virDomainSnapshotGetName(parent)); + ret = 0; goto cleanup; } if (last_error->code == VIR_ERR_NO_DOMAIN_SNAPSHOT) { /* API works, and we found a root with no parent */ + ret = 0; goto cleanup; } /* API didn't work, fall back to XML scraping. */ @@ -13054,15 +13067,23 @@ vshGetSnapshotParent(vshControl *ctl, virDomainSnapshotPtr snapshot) if (!xmldoc) goto cleanup; - parent_name = virXPathString("string(/domainsnapshot/parent/name)", ctxt); + *parent_name = virXPathString("string(/domainsnapshot/parent/name)", ctxt); + ret = 0; cleanup: + if (ret < 0) { + virshReportError(ctl); + vshError(ctl, "%s", _("unable to determine if snapshot has parent")); + } else { + virFreeError(last_error); + last_error = NULL; + } if (parent) virDomainSnapshotFree(parent); xmlXPathFreeContext(ctxt); xmlFreeDoc(xmldoc); VIR_FREE(xml); - return parent_name; + return ret; } /* @@ -13187,13 +13208,13 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) if (snapshot) virDomainSnapshotFree(snapshot); snapshot = virDomainSnapshotLookupByName(dom, names[i], 0); - if (!snapshot) { + if (!snapshot || + vshGetSnapshotParent(ctl, snapshot, &parents[i]) < 0) { while (--i >= 0) VIR_FREE(parents[i]); VIR_FREE(parents); goto cleanup; } - parents[i] = vshGetSnapshotParent(ctl, snapshot); } for (i = 0 ; i < actual ; i++) { memset(indentBuf, '\0', sizeof indentBuf); @@ -13389,9 +13410,12 @@ cmdSnapshotParent(vshControl *ctl, const vshCmd *cmd) if (snapshot == NULL) goto cleanup; - parent = vshGetSnapshotParent(ctl, snapshot); - if (!parent) + if (vshGetSnapshotParent(ctl, snapshot, &parent) < 0) goto cleanup; + if (!parent) { + vshError(ctl, _("snapshot '%s' has no parent"), name); + goto cleanup; + } vshPrint(ctl, "%s", parent); -- 1.7.4.4

2 2

[libvirt] Deadlock when using custom handlers
by Guido Günther 04 Oct '11

04 Oct '11

Hi, virt-viewer is using it's own virEventRegisterImpl. With current libvirt this can deadlock when connection to nonexistant URIs like qemu+ssh:///unknownhost.example.com/system like: 23:47:00.338: 1526: debug : doRemoteOpen:503 : proceeding with name = qemu:///system 23:47:00.338: 1526: debug : doRemoteOpen:513 : Connecting with transport 2 23:47:00.338: 1526: debug : virCommandRunAsync:2042 : About to run LC_ALL=C LD_LIBRARY_PATH=/var/scratch/debian/libvirt/upstream/libvirt/src/.libs/ PATH=/usr/local/bin:/usr/bin:/bin:/usr/games:/home/agx/bin:/sbin:/usr/sbin:/usr/lib/git-core HOME=/home/agx USER=agx LOGNAME=agx SSH_AUTH_SOCK=/tmp/keyring-RNvlnT/ssh DISPLAY=:0 ssh pustekiste nc -U /var/run/libvirt/libvirt-sock-ro 23:47:00.339: 1526: debug : virCommandRunAsync:2058 : Command result 0, with PID 1527 23:47:00.339: 1526: debug : virNetSocketNew:115 : localAddr=(nil) remoteAddr=(nil) fd=6 errfd=8 pid=1527 23:47:00.339: 1526: debug : virNetSocketNew:173 : sock=0x8926d20 localAddrStr=(null) remoteAddrStr=(null) ** (virt-viewer:1526): DEBUG: Add handle 6 1 0x8926d20 23:47:00.339: 1526: debug : virNetClientNew:160 : client=0xb5b5f008 refs=2 23:47:00.339: 1526: debug : doRemoteOpen:640 : Trying authentication 23:47:00.339: 1526: debug : virNetMessageNew:44 : msg=0xb5b1e008 23:47:00.340: 1526: debug : virNetMessageEncodePayload:255 : Encode length as 28 23:47:00.340: 1526: debug : virNetClientIO:1035 : program=536903814 version=1 serial=0 proc=66 type=0 length=28 dispatch=(nil) 23:47:00.340: 1526: debug : virNetClientIO:1103 : We have the buck 0x8939940 0x8939940 23:47:00.350: 1526: debug : virNetClientIOEventLoop:979 : Giving up the buck due to I/O error 0x8939940 (nil) 23:47:00.350: 1526: debug : virNetClientIO:1130 : All done with our call (nil) 0x8939940 -1 23:47:00.350: 1526: debug : virNetMessageFree:57 : msg=0xb5b1e008 ** (virt-viewer:1526): DEBUG: Remove handle 1 6 <deadlock> Gdb displays the deadlock nicely: #0 0xb7710424 in __kernel_vsyscall () #1 0xb692cf02 in __lll_lock_wait () at ../nptl/sysdeps/unix/sysv/linux/i386/i686/../i486/lowlevellock.S:142 #2 0xb692839b in _L_lock_728 () from /lib/i386-linux-gnu/i686/cmov/libpthread.so.0 #3 0xb69281c1 in __pthread_mutex_lock (mutex=0x95e1c30) at pthread_mutex_lock.c:61 #4 0xb698fd07 in virMutexLock (m=0x95e1c30) at util/threads-pthread.c:85 #5 0xb6a5dd56 in virNetSocketEventFree (opaque=0x95e1c30) at rpc/virnetsocket.c:1147 #6 0x080514a1 in virt_viewer_events_remove_handle (watch=1) at virt-viewer-events.c:178 #7 0xb697264e in virEventRemoveHandle (watch=1) at util/event.c:84 #8 0xb6a608ed in virNetSocketRemoveIOCallback (sock=0x95e1c30) at rpc/virnetsocket.c:1221 #9 0xb6a57114 in virNetClientClose (client=0xb5b7e008) at rpc/virnetclient.c:280 #10 0xb6a46fc4 in doRemoteOpen (conn=0x95e1aa8, priv=0x95b1370, auth=0xbfc3b178, flags=1) at remote/remote_driver.c:705 #11 0xb6a49612 in remoteOpen (conn=0x95e1aa8, auth=0xbfc3b178, flags=1) at remote/remote_driver.c:820 #12 0xb69f70c6 in do_open (name=0x95aac10 "qemu+ssh://pustekiste:2222/system", auth=0xbfc3b178, flags=1) at libvirt.c:1054 #13 0xb69f9b88 in virConnectOpenAuth (name=0x95aac10 "qemu+ssh://pustekiste:2222/system", auth=0xbfc3b178, flags=1) at libvirt.c:1280 #14 0x080500cf in virt_viewer_start (app=0x95a1010) at virt-viewer.c:483 #15 0x08053be8 in virt_viewer_app_start (self=0x95a1010) at virt-viewer-app.c:1044 #16 0x0804f54d in main (argc=1, argv=0xbfc3b3f4) at virt-viewer-main.c:120 which is the sock->lock: virNetSocketRemoveIOCallback holds sock->lock -> virEventRemoveHandle -> impl_remove_handle -> opaque->ff -> virNetSocketEventFree want to hold sock->lock Working around this by removing the locks from virNetSocketRemoveIOCallback leads to another deadlock: #0 0xb77de424 in __kernel_vsyscall () #1 0xb69faf02 in __lll_lock_wait () at ../nptl/sysdeps/unix/sysv/linux/i386/i686/../i486/lowlevellock.S:142 #2 0xb69f639b in _L_lock_728 () from /lib/i386-linux-gnu/i686/cmov/libpthread.so.0 #3 0xb69f61c1 in __pthread_mutex_lock (mutex=0xb5c4c00c) at pthread_mutex_lock.c:61 #4 0xb6a5dd07 in virMutexLock (m=0xb5c4c00c) at util/threads-pthread.c:85 #5 0xb6b24a98 in virNetClientLock (client=0xb5c4c008) at rpc/virnetclient.c:99 #6 virNetClientFree (client=0xb5c4c008) at rpc/virnetclient.c:243 #7 0xb6b250b7 in virNetClientEventFree (opaque=0xb5c4c008) at rpc/virnetclient.c:117 #8 0xb6b2bd82 in virNetSocketEventFree (opaque=0x8aabc30) at rpc/virnetsocket.c:1156 #9 0x080514a1 in virt_viewer_events_remove_handle (watch=1) at virt-viewer-events.c:178 #10 0xb6a4064e in virEventRemoveHandle (watch=1) at util/event.c:84 #11 0xb6b2e8e5 in virNetSocketRemoveIOCallback (sock=0x8aabc30) at rpc/virnetsocket.c:1219 #12 0xb6b25114 in virNetClientClose (client=0xb5c4c008) at rpc/virnetclient.c:280 #13 0xb6b14fc4 in doRemoteOpen (conn=0x8aabaa8, priv=0x8a7b370, auth=0xbfbf1798, flags=1) at remote/remote_driver.c:705 #14 0xb6b17612 in remoteOpen (conn=0x8aabaa8, auth=0xbfbf1798, flags=1) at remote/remote_driver.c:820 #15 0xb6ac50c6 in do_open (name=0x8a74c10 "qemu+ssh://pustekiste:2222/system", auth=0xbfbf1798, flags=1) at libvirt.c:1054 #16 0xb6ac7b88 in virConnectOpenAuth (name=0x8a74c10 "qemu+ssh://pustekiste:2222/system", auth=0xbfbf1798, flags=1) at libvirt.c:1280 #17 0x080500cf in virt_viewer_start (app=0x8a6b010) at virt-viewer.c:483 #18 0x08053be8 in virt_viewer_app_start (self=0x8a6b010) at virt-viewer-app.c:1044 #19 0x0804f54d in main (argc=1, argv=0xbfbf1a14) at virt-viewer-main.c:120 which is the virNetClient lock: virNetClientClose holds client->lock -> virNetSocketRemoveIOCallback -> virEventRemoveHandle -> impl_remove_handle -> virNetSocketEventFree -> virNetClientEventFree wants the lock I didn't see a simple way to fix this but would welcome any suggestions. Cheers, -- Guido

2 6

[libvirt] [PATCH] Make LXC work with new network configuration types
by Daniel P. Berrange 04 Oct '11

04 Oct '11

From: "Daniel P. Berrange" <berrange(a)redhat.com> If using one of the new non-NAT/routed virtual network configurations, the LXC driver would not know how to setup the VETH devices. Adding in calls to setup the "actual" network configuration at VM startup and cleanup when shutting down fixes this. * src/lxc/lxc_driver.c: Setup/cleanup actual net devs --- src/lxc/lxc_driver.c | 18 ++++++++++++++++-- 1 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index c8e6119..c475887 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -54,6 +54,7 @@ #include "fdstream.h" #include "domain_audit.h" #include "domain_nwfilter.h" +#include "network/bridge_driver.h" #define VIR_FROM_THIS VIR_FROM_LXC @@ -1042,6 +1043,8 @@ static void lxcVmCleanup(lxc_driver_t *driver, for (i = 0 ; i < vm->def->nnets ; i++) { vethInterfaceUpOrDown(vm->def->nets[i]->ifname, 0); vethDelete(vm->def->nets[i]->ifname); + + networkReleaseActualDevice(vm->def->nets[i]); } virDomainConfVMNWFilterTeardown(vm); @@ -1093,7 +1096,14 @@ static int lxcSetupInterfaces(virConnectPtr conn, char *parentVeth; char *containerVeth = NULL; - switch (def->nets[i]->type) { + /* If appropriate, grab a physical device from the configured + * network's pool of devices, or resolve bridge device name + * to the one defined in the network definition. + */ + if (networkAllocateActualDevice(def->nets[i]) < 0) + goto error_exit; + + switch (virDomainNetGetActualType(def->nets[i])) { case VIR_DOMAIN_NET_TYPE_NETWORK: { virNetworkPtr network; @@ -1110,7 +1120,7 @@ static int lxcSetupInterfaces(virConnectPtr conn, break; } case VIR_DOMAIN_NET_TYPE_BRIDGE: - bridge = def->nets[i]->data.bridge.brname; + bridge = virDomainNetGetActualBridgeName(def->nets[i]); break; case VIR_DOMAIN_NET_TYPE_USER: @@ -1183,6 +1193,10 @@ static int lxcSetupInterfaces(virConnectPtr conn, error_exit: brShutdown(brctl); + if (rc != 0) { + for (i = 0 ; i < def->nnets ; i++) + networkReleaseActualDevice(def->nets[i]); + } return rc; } -- 1.7.6.2

2 1

[libvirt] [PATCH] network: fill in bandwidth from portgroup for all forward modes
by Laine Stump 04 Oct '11

04 Oct '11

This patch is a fix for: https://bugzilla.redhat.com/show_bug.cgi?id=743176 which was discovered by Dan Berrange while making bandwidth configuration work for LXC guests. Background: Although virtportprofile data from a network portgroup is only applicable for direct mode interfaces, the code that copies bandwidth data from the portgroup was also only being executed in the case of direct mode interfaces. The result was that interfaces using traditional virtual networks (forward mode='nat|route|none'), and those using a host bridge for forwarding, would not pick up bandwidth data from a portgroup defined in the network. This patch moves that code outside the conditional, so that bandwidth information is *alway* copied from the appropriate portgroup (unless the <interface> definition itself already has bandwidth information, which would take precedence over what's in the portgroup anyway). --- src/conf/domain_conf.c | 6 +++- src/network/bridge_driver.c | 60 ++++++++++++++++++++++++++++-------------- 2 files changed, 44 insertions(+), 22 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6fb1888..ab1249b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2934,7 +2934,8 @@ virDomainActualNetDefParseXML(xmlNodePtr node, goto error; } if (actual->type != VIR_DOMAIN_NET_TYPE_BRIDGE && - actual->type != VIR_DOMAIN_NET_TYPE_DIRECT) { + actual->type != VIR_DOMAIN_NET_TYPE_DIRECT && + actual->type != VIR_DOMAIN_NET_TYPE_NETWORK) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported type '%s' in interface's <actual> element"), type); @@ -9406,7 +9407,8 @@ virDomainActualNetDefFormat(virBufferPtr buf, } if (def->type != VIR_DOMAIN_NET_TYPE_BRIDGE && - def->type != VIR_DOMAIN_NET_TYPE_DIRECT) { + def->type != VIR_DOMAIN_NET_TYPE_DIRECT && + def->type != VIR_DOMAIN_NET_TYPE_NETWORK) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected net type %s"), type); goto error; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 51023a3..445c3cb 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2753,6 +2753,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) struct network_driver *driver = driverState; virNetworkObjPtr network; virNetworkDefPtr netdef; + virPortGroupDefPtr portgroup; int ret = -1; if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK) @@ -2772,14 +2773,48 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) } netdef = network->def; - if ((netdef->forwardType == VIR_NETWORK_FORWARD_BRIDGE) && - netdef->bridge) { + + /* portgroup can be present for any type of network, in particular + * for bandwidth information, so we need to check for that and + * fill it in appropriately for all forward types. + */ + portgroup = virPortGroupFindByName(netdef, iface->data.network.portgroup); + + /* If there is already interface-specific bandwidth, just use that + * (already in NetDef). Otherwise, if there is bandwidth info in + * the portgroup, fill that into the ActualDef. + */ + if (portgroup && !iface->bandwidth) { + if (!iface->data.network.actual + && (VIR_ALLOC(iface->data.network.actual) < 0)) { + virReportOOMError(); + goto cleanup; + } + + if (virBandwidthCopy(&iface->data.network.actual->bandwidth, + portgroup->bandwidth) < 0) { + goto cleanup; + } + } + + if ((netdef->forwardType == VIR_NETWORK_FORWARD_NONE) || + (netdef->forwardType == VIR_NETWORK_FORWARD_NAT) || + (netdef->forwardType == VIR_NETWORK_FORWARD_ROUTE)) { + /* for these forward types, the actual net type really *is* + *NETWORK; we just keep the info from the portgroup in + * iface->data.network.actual + */ + if (iface->data.network.actual) + iface->data.network.actual->type = VIR_DOMAIN_NET_TYPE_NETWORK; + } else if ((netdef->forwardType == VIR_NETWORK_FORWARD_BRIDGE) && + netdef->bridge) { /* <forward type='bridge'/> <bridge name='xxx'/> * is VIR_DOMAIN_NET_TYPE_BRIDGE */ - if (VIR_ALLOC(iface->data.network.actual) < 0) { + if (!iface->data.network.actual + && (VIR_ALLOC(iface->data.network.actual) < 0)) { virReportOOMError(); goto cleanup; } @@ -2796,13 +2831,13 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) (netdef->forwardType == VIR_NETWORK_FORWARD_VEPA) || (netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH)) { virVirtualPortProfileParamsPtr virtport = NULL; - virPortGroupDefPtr portgroup = NULL; /* <forward type='bridge|private|vepa|passthrough'> are all * VIR_DOMAIN_NET_TYPE_DIRECT. */ - if (VIR_ALLOC(iface->data.network.actual) < 0) { + if (!iface->data.network.actual + && (VIR_ALLOC(iface->data.network.actual) < 0)) { virReportOOMError(); goto cleanup; } @@ -2825,7 +2860,6 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) } /* Find the most specific virtportprofile and copy it */ - portgroup = virPortGroupFindByName(netdef, iface->data.network.portgroup); if (iface->data.network.virtPortProfile) { virtport = iface->data.network.virtPortProfile; } else { @@ -2845,20 +2879,6 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) *iface->data.network.actual->data.direct.virtPortProfile = *virtport; } - /* Find the most specific bandwidth config and copy it */ - if (iface->bandwidth) { - if (virBandwidthCopy(&iface->data.network.actual->bandwidth, - iface->bandwidth) < 0) { - goto cleanup; - } - } else { - if (portgroup && - virBandwidthCopy(&iface->data.network.actual->bandwidth, - portgroup->bandwidth) < 0) { - goto cleanup; - } - } - /* If there is only a single device, just return it (caller will detect * any error if exclusive use is required but could not be acquired). */ -- 1.7.3.4

2 2

[libvirt] [RFC PATCH] lxc: don't return error on GetInfo when cgroups not yet set up
by Serge E. Hallyn 04 Oct '11

04 Oct '11

Nova (openstack) calls libvirt to create a container, then periodically checks using GetInfo to see whether the container is up. If it does this too quickly, then libvirt returns an error, which in libvirt.py causes an exception to be raised, the same type as if the container was bad. This may not be the best way to handle it, but with this patch, we assume that a -ENOENT return from virCgroupForDomain means the cgroups are not yet set up, and so we return the same values for cpu and memory usage as if the domain was not active. Signed-off-by: Serge Hallyn <serge.hallyn(a)canonical.com> --- src/lxc/lxc_driver.c | 37 +++++++++++++++++++++---------------- 1 files changed, 21 insertions(+), 16 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 4b62600..a68b8e7 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -542,26 +542,31 @@ static int lxcDomainGetInfo(virDomainPtr dom, info->cpuTime = 0; info->memory = vm->def->mem.cur_balloon; } else { - if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) != 0) { + int ret = virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0); + if (ret == -ENOENT) { + /* cgroups are not set up yet */ + info->cpuTime = 0; + info->memory = vm->def->mem.cur_balloon; + } else if (ret != 0) { lxcError(VIR_ERR_INTERNAL_ERROR, _("Unable to get cgroup for %s"), vm->def->name); goto cleanup; - } - - if (virCgroupGetCpuacctUsage(cgroup, &(info->cpuTime)) < 0) { - lxcError(VIR_ERR_OPERATION_FAILED, - "%s", _("Cannot read cputime for domain")); - goto cleanup; - } - if ((rc = virCgroupGetMemoryUsage(cgroup, &(info->memory))) < 0) { - lxcError(VIR_ERR_OPERATION_FAILED, - "%s", _("Cannot read memory usage for domain")); - if (rc == -ENOENT) { - /* Don't fail if we can't read memory usage due to a lack of - * kernel support */ - info->memory = 0; - } else + } else { + if (virCgroupGetCpuacctUsage(cgroup, &(info->cpuTime)) < 0) { + lxcError(VIR_ERR_OPERATION_FAILED, + "%s", _("Cannot read cputime for domain")); goto cleanup; + } + if ((rc = virCgroupGetMemoryUsage(cgroup, &(info->memory))) < 0) { + lxcError(VIR_ERR_OPERATION_FAILED, + "%s", _("Cannot read memory usage for domain")); + if (rc == -ENOENT) { + /* Don't fail if we can't read memory usage due to a lack of + * kernel support */ + info->memory = 0; + } else + goto cleanup; + } } } -- 1.7.5.4

3 10