- Devel - libvirt List Archives

[libvirt] [PATCHv2] qemu: Avoid holding the driver lock in trivial snapshot API's
by Peter Krempa 25 Sep '12

25 Sep '12

In most of the snapshot API's there's no need to hold the driver lock the whole time. This patch adds helper functions that get the domain object in functions that don't require the driver lock and simplifies call paths from snapshot-related API's. --- Diff to v1: - remove touching domain configuration locking to handle NULL gracefully and apropriate tweaks not to break code src/qemu/qemu_driver.c | 267 +++++++++++++++---------------------------------- 1 file changed, 79 insertions(+), 188 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6848924..af14197 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -187,6 +187,60 @@ struct qemuAutostartData { virConnectPtr conn; }; + +/* Looks up the domain obj and unlocks the driver */ +static virDomainObjPtr +qemuDomObjFromDomain(virDomainPtr domain) +{ + struct qemud_driver *driver = domain->conn->privateData; + virDomainObjPtr vm; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, domain->uuid); + qemuDriverUnlock(driver); + if (!vm) { + virUUIDFormat(domain->uuid, uuidstr); + virReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + } + + return vm; +} + + +/* Looks up domain obj from snapshot and unlocks the driver */ +static virDomainObjPtr +qemuDomObjFromSnapshot(virDomainSnapshotPtr snapshot) +{ + return qemuDomObjFromDomain(snapshot->domain); +} + + +/* Looks up snapshot obj from VM and name */ +static virDomainSnapshotObjPtr +qemuSnapObjFromName(virDomainObjPtr vm, + const char *name) +{ + virDomainSnapshotObjPtr snap = NULL; + snap = virDomainSnapshotFindByName(vm->snapshots, name); + if (!snap) + virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, + _("no domain snapshot with matching name '%s'"), + name); + + return snap; +} + + +/* Looks up snapshot obj from VM and snapshotPtr */ +static virDomainSnapshotObjPtr +qemuSnapObjFromSnapshot(virDomainObjPtr vm, + virDomainSnapshotPtr snapshot) +{ + return qemuSnapObjFromName(vm, snapshot->name); +} + static void qemuAutostartDomain(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaque) @@ -11305,22 +11359,14 @@ static int qemuDomainSnapshotListNames(virDomainPtr domain, char **names, int nameslen, unsigned int flags) { - struct qemud_driver *driver = domain->conn->privateData; virDomainObjPtr vm = NULL; int n = -1; virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_ROOTS | VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, domain->uuid); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(domain->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(domain))) goto cleanup; - } n = virDomainSnapshotObjListGetNames(vm->snapshots, NULL, names, nameslen, flags); @@ -11328,36 +11374,26 @@ static int qemuDomainSnapshotListNames(virDomainPtr domain, char **names, cleanup: if (vm) virDomainObjUnlock(vm); - qemuDriverUnlock(driver); return n; } static int qemuDomainSnapshotNum(virDomainPtr domain, unsigned int flags) { - struct qemud_driver *driver = domain->conn->privateData; virDomainObjPtr vm = NULL; int n = -1; virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_ROOTS | VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, domain->uuid); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(domain->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(domain))) goto cleanup; - } n = virDomainSnapshotObjListNum(vm->snapshots, NULL, flags); cleanup: if (vm) virDomainObjUnlock(vm); - qemuDriverUnlock(driver); return n; } @@ -11365,29 +11401,20 @@ static int qemuDomainListAllSnapshots(virDomainPtr domain, virDomainSnapshotPtr **snaps, unsigned int flags) { - struct qemud_driver *driver = domain->conn->privateData; virDomainObjPtr vm = NULL; int n = -1; virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_ROOTS | VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, domain->uuid); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(domain->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(domain))) goto cleanup; - } n = virDomainListSnapshots(vm->snapshots, NULL, domain, snaps, flags); cleanup: if (vm) virDomainObjUnlock(vm); - qemuDriverUnlock(driver); return n; } @@ -11397,7 +11424,6 @@ qemuDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot, int nameslen, unsigned int flags) { - struct qemud_driver *driver = snapshot->domain->conn->privateData; virDomainObjPtr vm = NULL; virDomainSnapshotObjPtr snap = NULL; int n = -1; @@ -11405,23 +11431,11 @@ qemuDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot, virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS | VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, snapshot->domain->uuid); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(snapshot->domain->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromSnapshot(snapshot))) goto cleanup; - } - snap = virDomainSnapshotFindByName(vm->snapshots, snapshot->name); - if (!snap) { - virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, - _("no domain snapshot with matching name '%s'"), - snapshot->name); + if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot))) goto cleanup; - } n = virDomainSnapshotObjListGetNames(vm->snapshots, snap, names, nameslen, flags); @@ -11429,7 +11443,6 @@ qemuDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot, cleanup: if (vm) virDomainObjUnlock(vm); - qemuDriverUnlock(driver); return n; } @@ -11437,7 +11450,6 @@ static int qemuDomainSnapshotNumChildren(virDomainSnapshotPtr snapshot, unsigned int flags) { - struct qemud_driver *driver = snapshot->domain->conn->privateData; virDomainObjPtr vm = NULL; virDomainSnapshotObjPtr snap = NULL; int n = -1; @@ -11445,30 +11457,17 @@ qemuDomainSnapshotNumChildren(virDomainSnapshotPtr snapshot, virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS | VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, snapshot->domain->uuid); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(snapshot->domain->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromSnapshot(snapshot))) goto cleanup; - } - snap = virDomainSnapshotFindByName(vm->snapshots, snapshot->name); - if (!snap) { - virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, - _("no domain snapshot with matching name '%s'"), - snapshot->name); + if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot))) goto cleanup; - } n = virDomainSnapshotObjListNum(vm->snapshots, snap, flags); cleanup: if (vm) virDomainObjUnlock(vm); - qemuDriverUnlock(driver); return n; } @@ -11477,7 +11476,6 @@ qemuDomainSnapshotListAllChildren(virDomainSnapshotPtr snapshot, virDomainSnapshotPtr **snaps, unsigned int flags) { - struct qemud_driver *driver = snapshot->domain->conn->privateData; virDomainObjPtr vm = NULL; virDomainSnapshotObjPtr snap = NULL; int n = -1; @@ -11485,23 +11483,11 @@ qemuDomainSnapshotListAllChildren(virDomainSnapshotPtr snapshot, virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS | VIR_DOMAIN_SNAPSHOT_FILTERS_ALL, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, snapshot->domain->uuid); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(snapshot->domain->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromSnapshot(snapshot))) goto cleanup; - } - snap = virDomainSnapshotFindByName(vm->snapshots, snapshot->name); - if (!snap) { - virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, - _("no domain snapshot with matching name '%s'"), - snapshot->name); + if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot))) goto cleanup; - } n = virDomainListSnapshots(vm->snapshots, snap, snapshot->domain, snaps, flags); @@ -11509,7 +11495,6 @@ qemuDomainSnapshotListAllChildren(virDomainSnapshotPtr snapshot, cleanup: if (vm) virDomainObjUnlock(vm); - qemuDriverUnlock(driver); return n; } @@ -11517,64 +11502,41 @@ static virDomainSnapshotPtr qemuDomainSnapshotLookupByName(virDomainPtr domain, const char *name, unsigned int flags) { - struct qemud_driver *driver = domain->conn->privateData; virDomainObjPtr vm; virDomainSnapshotObjPtr snap = NULL; virDomainSnapshotPtr snapshot = NULL; virCheckFlags(0, NULL); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, domain->uuid); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(domain->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(domain))) goto cleanup; - } - snap = virDomainSnapshotFindByName(vm->snapshots, name); - if (!snap) { - virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, - _("no snapshot with matching name '%s'"), name); + if (!(snap = qemuSnapObjFromName(vm, name))) goto cleanup; - } snapshot = virGetDomainSnapshot(domain, snap->def->name); cleanup: - if (vm) - virDomainObjUnlock(vm); - qemuDriverUnlock(driver); + virDomainObjUnlock(vm); return snapshot; } static int qemuDomainHasCurrentSnapshot(virDomainPtr domain, unsigned int flags) { - struct qemud_driver *driver = domain->conn->privateData; virDomainObjPtr vm; int ret = -1; virCheckFlags(0, -1); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, domain->uuid); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(domain->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(domain))) goto cleanup; - } ret = (vm->current_snapshot != NULL); cleanup: if (vm) virDomainObjUnlock(vm); - qemuDriverUnlock(driver); return ret; } @@ -11582,30 +11544,17 @@ static virDomainSnapshotPtr qemuDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, unsigned int flags) { - struct qemud_driver *driver = snapshot->domain->conn->privateData; virDomainObjPtr vm; virDomainSnapshotObjPtr snap = NULL; virDomainSnapshotPtr parent = NULL; virCheckFlags(0, NULL); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, snapshot->domain->uuid); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(snapshot->domain->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromSnapshot(snapshot))) goto cleanup; - } - snap = virDomainSnapshotFindByName(vm->snapshots, snapshot->name); - if (!snap) { - virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, - _("no domain snapshot with matching name '%s'"), - snapshot->name); + if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot))) goto cleanup; - } if (!snap->def->parent) { virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, @@ -11619,28 +11568,19 @@ qemuDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, cleanup: if (vm) virDomainObjUnlock(vm); - qemuDriverUnlock(driver); return parent; } static virDomainSnapshotPtr qemuDomainSnapshotCurrent(virDomainPtr domain, unsigned int flags) { - struct qemud_driver *driver = domain->conn->privateData; virDomainObjPtr vm; virDomainSnapshotPtr snapshot = NULL; virCheckFlags(0, NULL); - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, domain->uuid); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(domain->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(domain))) goto cleanup; - } if (!vm->current_snapshot) { virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, "%s", @@ -11653,14 +11593,12 @@ static virDomainSnapshotPtr qemuDomainSnapshotCurrent(virDomainPtr domain, cleanup: if (vm) virDomainObjUnlock(vm); - qemuDriverUnlock(driver); return snapshot; } static char *qemuDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, unsigned int flags) { - struct qemud_driver *driver = snapshot->domain->conn->privateData; virDomainObjPtr vm = NULL; char *xml = NULL; virDomainSnapshotObjPtr snap = NULL; @@ -11668,29 +11606,19 @@ static char *qemuDomainSnapshotGetXMLDesc(virDomainSnapshotPtr snapshot, virCheckFlags(VIR_DOMAIN_XML_SECURE, NULL); - qemuDriverLock(driver); - virUUIDFormat(snapshot->domain->uuid, uuidstr); - vm = virDomainFindByUUID(&driver->domains, snapshot->domain->uuid); - if (!vm) { - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromSnapshot(snapshot))) goto cleanup; - } - snap = virDomainSnapshotFindByName(vm->snapshots, snapshot->name); - if (!snap) { - virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, - _("no domain snapshot with matching name '%s'"), - snapshot->name); + if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot))) goto cleanup; - } + + virUUIDFormat(snapshot->domain->uuid, uuidstr); xml = virDomainSnapshotDefFormat(uuidstr, snap->def, flags, 0); cleanup: if (vm) virDomainObjUnlock(vm); - qemuDriverUnlock(driver); return xml; } @@ -11698,30 +11626,17 @@ static int qemuDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot, unsigned int flags) { - struct qemud_driver *driver = snapshot->domain->conn->privateData; virDomainObjPtr vm = NULL; int ret = -1; virDomainSnapshotObjPtr snap = NULL; - char uuidstr[VIR_UUID_STRING_BUFLEN]; virCheckFlags(0, -1); - qemuDriverLock(driver); - virUUIDFormat(snapshot->domain->uuid, uuidstr); - vm = virDomainFindByUUID(&driver->domains, snapshot->domain->uuid); - if (!vm) { - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromSnapshot(snapshot))) goto cleanup; - } - snap = virDomainSnapshotFindByName(vm->snapshots, snapshot->name); - if (!snap) { - virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, - _("no domain snapshot with matching name '%s'"), - snapshot->name); + if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot))) goto cleanup; - } ret = (vm->current_snapshot && STREQ(snapshot->name, vm->current_snapshot->def->name)); @@ -11729,7 +11644,6 @@ qemuDomainSnapshotIsCurrent(virDomainSnapshotPtr snapshot, cleanup: if (vm) virDomainObjUnlock(vm); - qemuDriverUnlock(driver); return ret; } @@ -11738,30 +11652,17 @@ static int qemuDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot, unsigned int flags) { - struct qemud_driver *driver = snapshot->domain->conn->privateData; virDomainObjPtr vm = NULL; int ret = -1; virDomainSnapshotObjPtr snap = NULL; - char uuidstr[VIR_UUID_STRING_BUFLEN]; virCheckFlags(0, -1); - qemuDriverLock(driver); - virUUIDFormat(snapshot->domain->uuid, uuidstr); - vm = virDomainFindByUUID(&driver->domains, snapshot->domain->uuid); - if (!vm) { - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromSnapshot(snapshot))) goto cleanup; - } - snap = virDomainSnapshotFindByName(vm->snapshots, snapshot->name); - if (!snap) { - virReportError(VIR_ERR_NO_DOMAIN_SNAPSHOT, - _("no domain snapshot with matching name '%s'"), - snapshot->name); + if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot))) goto cleanup; - } /* XXX Someday, we should recognize internal snapshots in qcow2 * images that are not tied to a libvirt snapshot; if we ever do @@ -11771,7 +11672,6 @@ qemuDomainSnapshotHasMetadata(virDomainSnapshotPtr snapshot, cleanup: if (vm) virDomainObjUnlock(vm); - qemuDriverUnlock(driver); return ret; } @@ -12439,9 +12339,7 @@ qemuDomainOpenConsole(virDomainPtr dom, virStreamPtr st, unsigned int flags) { - struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm = NULL; - char uuidstr[VIR_UUID_STRING_BUFLEN]; int ret = -1; int i; virDomainChrDefPtr chr = NULL; @@ -12450,15 +12348,8 @@ qemuDomainOpenConsole(virDomainPtr dom, virCheckFlags(VIR_DOMAIN_CONSOLE_SAFE | VIR_DOMAIN_CONSOLE_FORCE, -1); - qemuDriverLock(driver); - virUUIDFormat(dom->uuid, uuidstr); - vm = virDomainFindByUUID(&driver->domains, dom->uuid); - qemuDriverUnlock(driver); - if (!vm) { - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - } if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, -- 1.7.12

2 2

[libvirt] [libvirt-glib] Don't call g_set_error with a NULL format string
by Christophe Fergeau 25 Sep '12

25 Sep '12

From: Jovanka Gulicoska <jovanka.gulicoska(a)gmail.com> The format string passed to g_set_error cannot be NULL or a runtime warning will be printed. --- libvirt-gobject/libvirt-gobject-stream.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-stream.c b/libvirt-gobject/libvirt-gobject-stream.c index 3b98023..fa1a32c 100644 --- a/libvirt-gobject/libvirt-gobject-stream.c +++ b/libvirt-gobject/libvirt-gobject-stream.c @@ -319,7 +319,8 @@ gssize gvir_stream_receive(GVirStream *self, got = virStreamRecv(self->priv->handle, buffer, size); if (got == -2) { /* blocking */ - g_set_error(error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK, NULL); + g_set_error_literal(error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK, + "virStreamRecv call would block"); } else if (got < 0) { g_set_error(error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT, "Got virStreamRecv error in %s", G_STRFUNC); @@ -435,7 +436,8 @@ gssize gvir_stream_send(GVirStream *self, got = virStreamSend(self->priv->handle, buffer, size); if (got == -2) { /* blocking */ - g_set_error(error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK, NULL); + g_set_error_literal(error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK, + "virStreamSend call would block"); } else if (got < 0) { g_set_error(error, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT, "Got virStreamRecv error in %s", G_STRFUNC); -- 1.7.11.4

2 2

[libvirt] [PATCH v1 0/2] Fix environment-setting code.
by Richard W.M. Jones 25 Sep '12

25 Sep '12

[This patch is for discussion only: I have not proven that it fixes the bug] https://bugzilla.redhat.com/show_bug.cgi?id=859596 When libvirtd runs any command, it uses a simple strategy to set the environment for that command: (a) It creates a new, empty environment array. (b) It copies in a few known-good environment variables, including TMPDIR. (c) It appends environment variables specified by the user, eg. ones specified in <qemu:env>. Unfortunately this means if you try to set TMPDIR via a <qemu:env> XML directive, then it is appended to the list of environment variables which already includes TMPDIR. The qemu subprocess picks an environment variable (probably) first from the list, which is the TMPDIR that libvirtd had originally, not the TMPDIR that was specified by the user. This [possibly, not proven] leads to bug 859596, where we see qemu trying to create temporary files in a directory that no longer exists. The follow-up patches change libvirtd so that environment variables in the virCommandPtr array are replaced if they exist already. Rich.

2 11

[libvirt] [PATCH V1 0/2] Add support for IP layer 4 protocol filtering
by Stefan Berger 25 Sep '12

25 Sep '12

The 1st patch in this series extends the list of IP layer 4 protocol name-to-identifier translations. The 2nd patch closes a gap in the possibility to filter layer 4 protocols' layer 2 and 3 headers. This patch makes the 'iptables --protocol <protocol>' parameter accessible using an ipl4 XML node. Stefan

1 3

[libvirt] [PATCH v2 0/2] security: support for names on DAC labels
by Marcelo Cerri 25 Sep '12

25 Sep '12

This patch series extends label parsing for DAC security driver and updates the related documentation. Marcelo Cerri (2): security: also parse user/group names instead of just IDs for DAC labels doc: update description about security labels on formatdomain.html docs/formatdomain.html.in | 14 ++++++++-- src/security/security_dac.c | 62 +++++++++++++++++++++++++++++++++++++++------ 2 files changed, 66 insertions(+), 10 deletions(-) -- 1.7.12

2 5

[libvirt] [PATCH] storage: Add timeout for iscsi volume's stable path discovery
by Osier Yang 25 Sep '12

25 Sep '12

It might need some time till the LUN's stable path shows up on initiator host, and although the time window is not foreseeable, as a better than nothing fix, this patch adds timeout for the stable path discovery process. --- src/storage/storage_backend.c | 14 ++++++++++++-- 1 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 1d32232..e159bd2 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1347,6 +1347,7 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool, struct dirent *dent; char *stablepath; int opentries = 0; + int retry = 0; /* Short circuit if pool has no target, or if its /dev */ if (pool->def->target.path == NULL || @@ -1355,7 +1356,7 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool, goto ret_strdup; /* Skip whole thing for a pool which isn't in /dev - * so we don't mess will filesystem/dir based pools + * so we don't mess filesystem/dir based pools */ if (!STRPREFIX(pool->def->target.path, "/dev")) goto ret_strdup; @@ -1384,8 +1385,12 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool, /* The pool is pointing somewhere like /dev/disk/by-path * or /dev/disk/by-id, so we need to check all symlinks in * the target directory and figure out which one points - * to this device node + * to this device node. + * + * And it might need some time till the stabe path shows + * up, so add timeout to retry here. */ + retry: while ((dent = readdir(dh)) != NULL) { if (dent->d_name[0] == '.') continue; @@ -1406,6 +1411,11 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool, VIR_FREE(stablepath); } + if (++retry < 100) { + usleep(100 * 1000); + goto retry; + } + closedir(dh); ret_strdup: -- 1.7.7.3

2 2

[libvirt] [libvirt-designer][PATCH 0/6] Another functional extension
by Michal Privoznik 25 Sep '12

25 Sep '12

and couple of fixes for clean RPM build. Michal Privoznik (6): Implement resources setting Create basic documentation domain.c: Document @error on public APIs spec: Remove unused rules Create manpage for virtxml spec: Cleanup for rpmlint .gitignore | 17 ++++ Makefile.am | 2 +- configure.ac | 1 + docs/Makefile.am | 42 ++++++++++ docs/libvirt-designer-docs.xml | 27 ++++++ examples/Makefile.am | 11 +++ examples/virtxml.c | 23 +++++ examples/virtxml.pod | 121 ++++++++++++++++++++++++++++ libvirt-designer.spec.in | 31 ++----- libvirt-designer/libvirt-designer-domain.c | 95 ++++++++++++++++++++++ libvirt-designer/libvirt-designer-domain.h | 10 +++ libvirt-designer/libvirt-designer.sym | 2 + 12 files changed, 358 insertions(+), 24 deletions(-) create mode 100644 docs/Makefile.am create mode 100644 docs/libvirt-designer-docs.xml create mode 100644 examples/virtxml.pod -- 1.7.8.6

3 15

[libvirt] RFC: Check if the host device is assignable earlier
by Osier Yang 25 Sep '12

25 Sep '12

Hi, I think we already got enough bugs which are about "try to assign a host device, then it fails for not assignable", clearly this "try and then fails" approach is not nice. The reason for not assignable can be various. E.g. * IOMMU or AMD-V is not enabled/supported * platform doesn't support interrupt remapping There is no checking for above reasons now, an assignment will get error till the qemu process started. It's nice if checking it much earlier, and list whether the host supports device assignment as a property of capabilities? * The device can't be unbind/reset * Is being used by some other guests * Permission issues * Other reasons? It will also be nice if one could known what devices are assignable (list all assignable devices? this needs to enumerate and try to unbind/reset/restore the devices, which is not desireable, as it could breaks work on host, such as NIC). Or whether a device is assignable (this shouldn't cause any problem when doing checking, because it's the requested operation, one knowns what he does). Assuming a new API to determine whether a host device is assignable for qemu driver, the workflow for the implementation should be: * If the device is being used (Examine activePciHostdevs, or activeUsbHostdevs) * If there is permission issues * Other possible reasons should be checked before trying to unbind and reset. * If the device can be unbind/reset It's basicly a rough thought, any idea is welcomed. Regards, Osier

2 2

[libvirt] [PATCH] node_memory: Improve the docs
by Osier Yang 25 Sep '12

25 Sep '12

To declare that it's not recommended to use the tuning API while the outside tuning program (such as ksmtuned under Linux) is running. --- src/libvirt.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 0cbcfbf..43591c0 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -6794,6 +6794,12 @@ error: * @flags: extra flags; not used yet, so callers should always pass 0 * * Change all or a subset of the node memory tunables. + * + * Note that it's not recommended to use this function while the + * outside tuning program is running (such as ksmtuned under Linux), + * as they could change the tunables in parallel, which could cause + * conflicts. + * * This function may require privileged access to the hypervisor. * * Returns 0 in case of success, -1 in case of failure. -- 1.7.7.3

2 2

[libvirt] [PATCHv4] interface: add udev based backend for virInterface
by Doug Goldstein 25 Sep '12

25 Sep '12

Add a read-only udev based backend for virInterface. Useful for distros that do not have netcf support yet. Multiple libvirt based utilities use a HAL based fallback when virInterface is not available which is less than ideal. This implements: * virConnectNumOfInterfaces() * virConnectListInterfaces() * virConnectNumOfDefinedInterfaces() * virConnectListDefinedInterfaces() * virConnectInterfaceLookupByName() * virConnectInterfaceLookupByMACString() --- Change from v3: * fix make syntax-check issues * check strdup() and handle that case * simplify configure check Change from v2: * rebase from master Change from v1: * rebase from master configure.ac | 18 +- po/POTFILES.in | 1 + src/Makefile.am | 12 +- src/interface/interface_backend_udev.c | 430 ++++++++++++++++++++++++++++++++ tools/virsh.c | 2 + 5 files changed, 450 insertions(+), 13 deletions(-) create mode 100644 src/interface/interface_backend_udev.c diff --git a/configure.ac b/configure.ac index 3e9073d..a3d9ab1 100644 --- a/configure.ac +++ b/configure.ac @@ -2798,18 +2798,12 @@ if test "$with_libvirtd" = "no" ; then with_interface=no fi -dnl The interface driver depends on the netcf library -if test "$with_interface:$with_netcf" = "check:yes" ; then - with_interface=yes -fi - -if test "$with_interface:$with_netcf" = "check:no" ; then - with_interface=no -fi - -if test "$with_interface:$with_netcf" = "yes:no" ; then - AC_MSG_ERROR([Requested the Interface driver without netcf support]) -fi +dnl The interface driver depends on the netcf library or udev library +case $with_interface:$with_netcf:$with_udev in + check:*yes*) with_interface=yes ;; + check:no:no) with_interface=no ;; + yes:no:no) AC_MSG_ERROR([Requested the Interface driver without netcf or udev support]) ;; +esac if test "$with_interface" = "yes" ; then AC_DEFINE_UNQUOTED([WITH_INTERFACE], [1], diff --git a/po/POTFILES.in b/po/POTFILES.in index 12a2b25..0999260 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -41,6 +41,7 @@ src/hyperv/hyperv_driver.c src/hyperv/hyperv_util.c src/hyperv/hyperv_wmi.c src/interface/interface_backend_netcf.c +src/interface/interface_backend_udev.c src/internal.h src/libvirt.c src/libvirt-qemu.c diff --git a/src/Makefile.am b/src/Makefile.am index 4ae741b..53f1fbf 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -557,11 +557,16 @@ INTERFACE_DRIVER_SOURCES = if WITH_INTERFACE INTERFACE_DRIVER_SOURCES += \ interface/interface_driver.h -endif if WITH_NETCF INTERFACE_DRIVER_SOURCES += \ interface/interface_backend_netcf.c +else +if HAVE_UDEV +INTERFACE_DRIVER_SOURCES += \ + interface/interface_backend_udev.c +endif +endif endif SECRET_DRIVER_SOURCES = \ @@ -1044,6 +1049,11 @@ libvirt_driver_interface_la_LIBADD = if WITH_NETCF libvirt_driver_interface_la_CFLAGS += $(NETCF_CFLAGS) libvirt_driver_interface_la_LIBADD += $(NETCF_LIBS) +else +if HAVE_UDEV +libvirt_driver_interface_la_CFLAGS += $(UDEV_CFLAGS) +libvirt_driver_interface_la_LIBADD += $(UDEV_LIBS) +endif endif if WITH_DRIVER_MODULES libvirt_driver_interface_la_LIBADD += ../gnulib/lib/libgnu.la diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c new file mode 100644 index 0000000..a823d2d --- /dev/null +++ b/src/interface/interface_backend_udev.c @@ -0,0 +1,430 @@ +/* + * interface_backend_udev.c: udev backend for virInterface + * + * Copyright (C) 2012 Doug Goldstein <cardoe(a)cardoe.com> + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * <http://www.gnu.org/licenses/>. + */ +#include <config.h> + +#include <libudev.h> + +#include "virterror_internal.h" +#include "datatypes.h" +#include "interface_driver.h" +#include "interface_conf.h" +#include "memory.h" + +#define VIR_FROM_THIS VIR_FROM_INTERFACE + +struct udev_iface_driver { + struct udev *udev; +}; + +enum udev_status { UDEV_IFACE_ACTIVE, UDEV_IFACE_INACTIVE, UDEV_IFACE_ALL }; + +static struct udev_enumerate * +udevIfaceGetDevices(struct udev *udev, enum udev_status status) +{ + struct udev_enumerate *enumerate; + + /* Check input params for sanity */ + if (!udev) + return NULL; + + /* Create a new enumeration to create a list */ + enumerate = udev_enumerate_new(udev); + + if (!enumerate) + return NULL; + + /* Enumerate all network subsystem devices */ + udev_enumerate_add_match_subsystem(enumerate, "net"); + + /* Ignore devices that are part of a bridge */ + udev_enumerate_add_nomatch_sysattr(enumerate, "brport/state", NULL); + + /* State of the device */ + switch (status) { + case UDEV_IFACE_ACTIVE: + udev_enumerate_add_match_sysattr(enumerate, "operstate", "up"); + break; + + case UDEV_IFACE_INACTIVE: + udev_enumerate_add_match_sysattr(enumerate, "operstate", "down"); + break; + + case UDEV_IFACE_ALL: + break; + } + + /* We don't want to see the TUN devices that QEMU creates for other gets + * running on this machine. By saying nomatch NULL, we just are getting + * devices without the tun_flags sysattr. + */ + udev_enumerate_add_nomatch_sysattr(enumerate, "tun_flags", NULL); + + return enumerate; +} + +static virDrvOpenStatus +udevIfaceOpenInterface(virConnectPtr conn, + virConnectAuthPtr auth ATTRIBUTE_UNUSED, + unsigned int flags) +{ + struct udev_iface_driver *driverState = NULL; + + virCheckFlags(0, VIR_DRV_OPEN_ERROR); + + if (VIR_ALLOC(driverState) < 0) { + virReportOOMError(); + goto err; + } + + driverState->udev = udev_new(); + if (!driverState->udev) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to create udev context")); + goto err; + } + + conn->interfacePrivateData = driverState; + + return VIR_DRV_OPEN_SUCCESS; + +err: + VIR_FREE(driverState); + + return VIR_DRV_OPEN_ERROR; +} + +static int +udevIfaceCloseInterface(virConnectPtr conn) +{ + struct udev_iface_driver *driverState; + + if (conn->interfacePrivateData != NULL) { + driverState = conn->interfacePrivateData; + + udev_unref(driverState->udev); + + VIR_FREE(driverState); + } + + conn->interfacePrivateData = NULL; + return 0; +} + +static int +udevIfaceNumOfInterfaces(virConnectPtr conn) +{ + struct udev_iface_driver *driverState = conn->interfacePrivateData; + struct udev *udev = udev_ref(driverState->udev); + struct udev_enumerate *enumerate = NULL; + struct udev_list_entry *devices; + struct udev_list_entry *dev_entry; + int count = 0; + + enumerate = udevIfaceGetDevices(udev, UDEV_IFACE_ACTIVE); + + if (!enumerate) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to get number of active interfaces on host")); + return -1; + } + + /* Do the scan to load up the enumeration */ + udev_enumerate_scan_devices(enumerate); + + /* Get a list we can walk */ + devices = udev_enumerate_get_list_entry(enumerate); + + /* For each item so we can count */ + udev_list_entry_foreach(dev_entry, devices) { + count++; + } + + udev_enumerate_unref(enumerate); + + udev_unref(udev); + + return count; +} + +static int +udevIfaceListInterfaces(virConnectPtr conn, char **const names, int names_len) +{ + struct udev_iface_driver *driverState = conn->interfacePrivateData; + struct udev *udev = udev_ref(driverState->udev); + struct udev_enumerate *enumerate = NULL; + struct udev_list_entry *devices; + struct udev_list_entry *dev_entry; + int count = 0; + + enumerate = udevIfaceGetDevices(udev, UDEV_IFACE_ACTIVE); + + if (!enumerate) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to get list of active interfaces on host")); + return -1; + } + + /* Do the scan to load up the enumeration */ + udev_enumerate_scan_devices(enumerate); + + /* Get a list we can walk */ + devices = udev_enumerate_get_list_entry(enumerate); + + /* For each item so we can count */ + udev_list_entry_foreach(dev_entry, devices) { + struct udev_device *dev; + const char *path; + + /* Ensure we won't exceed the size of our array */ + if (count > names_len) + break; + + path = udev_list_entry_get_name(dev_entry); + dev = udev_device_new_from_syspath(udev, path); + names[count] = strdup(udev_device_get_sysname(dev)); + udev_device_unref(dev); + + /* If strdup() failed, we are out of memory */ + if (!names[count]) { + virReportOOMError(); + goto err; + } + + count++; + } + + udev_enumerate_unref(enumerate); + udev_unref(udev); + + return count; + +err: + udev_enumerate_unref(enumerate); + udev_unref(udev); + + for (names_len = 0; names_len < count; names_len++) + VIR_FREE(names[names_len]); + + return -1; +} + +static int +udevIfaceNumOfDefinedInterfaces(virConnectPtr conn) +{ + struct udev_iface_driver *driverState = conn->interfacePrivateData; + struct udev *udev = udev_ref(driverState->udev); + struct udev_enumerate *enumerate = NULL; + struct udev_list_entry *devices; + struct udev_list_entry *dev_entry; + int count = 0; + + enumerate = udevIfaceGetDevices(udev, UDEV_IFACE_INACTIVE); + + if (!enumerate) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to get number of defined interfaces on host")); + return -1; + } + + /* Do the scan to load up the enumeration */ + udev_enumerate_scan_devices(enumerate); + + /* Get a list we can walk */ + devices = udev_enumerate_get_list_entry(enumerate); + + /* For each item so we can count */ + udev_list_entry_foreach(dev_entry, devices) { + count++; + } + + udev_enumerate_unref(enumerate); + + udev_unref(udev); + + return count; +} + +static int +udevIfaceListDefinedInterfaces(virConnectPtr conn, + char **const names, + int names_len) +{ + struct udev_iface_driver *driverState = conn->interfacePrivateData; + struct udev *udev = udev_ref(driverState->udev); + struct udev_enumerate *enumerate = NULL; + struct udev_list_entry *devices; + struct udev_list_entry *dev_entry; + int count = 0; + + enumerate = udevIfaceGetDevices(udev, UDEV_IFACE_INACTIVE); + + if (!enumerate) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to get list of defined interfaces on host")); + return -1; + } + + /* Do the scan to load up the enumeration */ + udev_enumerate_scan_devices(enumerate); + + /* Get a list we can walk */ + devices = udev_enumerate_get_list_entry(enumerate); + + /* For each item so we can count */ + udev_list_entry_foreach(dev_entry, devices) { + struct udev_device *dev; + const char *path; + + /* Ensure we won't exceed the size of our array */ + if (count > names_len) + break; + + path = udev_list_entry_get_name(dev_entry); + dev = udev_device_new_from_syspath(udev, path); + names[count] = strdup(udev_device_get_sysname(dev)); + udev_device_unref(dev); + + /* If strdup() failed, we are out of memory */ + if (!names[count]) { + virReportOOMError(); + goto err; + } + + count++; + } + + udev_enumerate_unref(enumerate); + udev_unref(udev); + + return count; + +err: + udev_enumerate_unref(enumerate); + udev_unref(udev); + + for (names_len = 0; names_len < count; names_len++) + VIR_FREE(names[names_len]); + + return -1; +} + +static virInterfacePtr +udevIfaceLookupByName(virConnectPtr conn, const char *name) +{ + struct udev_iface_driver *driverState = conn->interfacePrivateData; + struct udev *udev = udev_ref(driverState->udev); + struct udev_device *dev; + const char *macaddr; + virInterfacePtr ret; + + /* get a device reference based on the device name */ + dev = udev_device_new_from_subsystem_sysname(udev, "net", name); + if (!dev) { + virReportError(VIR_ERR_NO_INTERFACE, + _("couldn't find interface named '%s'"), + name); + return NULL; + } + + macaddr = udev_device_get_sysattr_value(dev, "address"); + ret = virGetInterface(conn, name, macaddr); + udev_device_unref(dev); + + udev_unref(udev); + + return ret; +} + +static virInterfacePtr +udevIfaceLookupByMACString(virConnectPtr conn, const char *macstr) +{ + struct udev_iface_driver *driverState = conn->interfacePrivateData; + struct udev *udev = udev_ref(driverState->udev); + struct udev_enumerate *enumerate = NULL; + struct udev_list_entry *dev_entry; + struct udev_device *dev; + const char *name; + virInterfacePtr ret; + + enumerate = udevIfaceGetDevices(udev, UDEV_IFACE_ALL); + + if (!enumerate) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to lookup interface with MAC address '%s'"), + macstr); + return NULL; + } + + /* Match on MAC */ + udev_enumerate_add_match_sysattr(enumerate, "address", macstr); + + /* Do the scan to load up the enumeration */ + udev_enumerate_scan_devices(enumerate); + + /* Get a list we can walk */ + dev_entry = udev_enumerate_get_list_entry(enumerate); + + /* Check that we got something back */ + if (!dev_entry) { + virReportError(VIR_ERR_NO_INTERFACE, + _("couldn't find interface with MAC address '%s'"), + macstr); + return NULL; + } + + /* Check that we didn't get multiple items back */ + if (udev_list_entry_get_next(dev_entry)) { + virReportError(VIR_ERR_MULTIPLE_INTERFACES, + _("the MAC address '%s' matches multiple interfaces"), + macstr); + return NULL; + } + + dev = udev_device_new_from_syspath(udev, udev_list_entry_get_name(dev_entry)); + name = udev_device_get_sysname(dev); + ret = virGetInterface(conn, name, macstr); + udev_device_unref(dev); + + udev_enumerate_unref(enumerate); + + udev_unref(udev); + + return ret; +} + +static virInterfaceDriver udevIfaceDriver = { + "Interface", + .open = udevIfaceOpenInterface, /* 0.10.3 */ + .close = udevIfaceCloseInterface, /* 0.10.3 */ + .numOfInterfaces = udevIfaceNumOfInterfaces, /* 0.10.3 */ + .listInterfaces = udevIfaceListInterfaces, /* 0.10.3 */ + .numOfDefinedInterfaces = udevIfaceNumOfDefinedInterfaces, /* 0.10.3 */ + .listDefinedInterfaces = udevIfaceListDefinedInterfaces, /* 0.10.3 */ + .interfaceLookupByName = udevIfaceLookupByName, /* 0.10.3 */ + .interfaceLookupByMACString = udevIfaceLookupByMACString, /* 0.10.3 */ +}; + +int +interfaceRegister(void) { + if (virRegisterInterfaceDriver(&udevIfaceDriver) < 0) + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to register udev interface driver")); + return 0; +} diff --git a/tools/virsh.c b/tools/virsh.c index b32ee8b..f0ec625 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2708,6 +2708,8 @@ vshShowVersion(vshControl *ctl ATTRIBUTE_UNUSED) vshPrint(ctl, " Interface"); # if defined(WITH_NETCF) vshPrint(ctl, " netcf"); +# elif defined(HAVE_UDEV) + vshPrint(ctl, " udev"); # endif #endif #ifdef WITH_NWFILTER -- 1.7.8.6

1 0