- Devel - libvirt List Archives

[libvirt] [RFC PATCH 1/5] Add new API virDomainBlockIoThrottle
by Lei Li 13 Oct '11

13 Oct '11

Signed-off-by: Zhi Yong Wu <wuzhy(a)linux.vnet.ibm.com> --- include/libvirt/libvirt.h.in | 22 ++++++++++++ src/conf/domain_conf.c | 77 ++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 11 ++++++ src/driver.h | 11 ++++++ src/libvirt.c | 66 ++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + src/util/xml.h | 3 ++ 7 files changed, 191 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 07617be..f7b892d 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1573,6 +1573,28 @@ int virDomainBlockJobSetSpeed(virDomainPtr dom, const char *path, int virDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth, unsigned int flags); +/* + * Block I/O throttling support + */ + +typedef unsigned long long virDomainBlockIoThrottleUnits; + +typedef struct _virDomainBlockIoThrottleInfo virDomainBlockIoThrottleInfo; +struct _virDomainBlockIoThrottleInfo { + virDomainBlockIoThrottleUnits bps; + virDomainBlockIoThrottleUnits bps_rd; + virDomainBlockIoThrottleUnits bps_wr; + virDomainBlockIoThrottleUnits iops; + virDomainBlockIoThrottleUnits iops_rd; + virDomainBlockIoThrottleUnits iops_wr; +}; +typedef virDomainBlockIoThrottleInfo *virDomainBlockIoThrottleInfoPtr; + +int virDomainBlockIoThrottle(virDomainPtr dom, + const char *disk, + virDomainBlockIoThrottleInfoPtr info, + virDomainBlockIoThrottleInfoPtr reply, + unsigned int flags); /* * NUMA support diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 944cfa9..d0ba07e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2422,6 +2422,42 @@ virDomainDiskDefParseXML(virCapsPtr caps, iotag = virXMLPropString(cur, "io"); ioeventfd = virXMLPropString(cur, "ioeventfd"); event_idx = virXMLPropString(cur, "event_idx"); + } else if (xmlStrEqual(cur->name, BAD_CAST "iothrottle")) { + char *io_throttle = NULL; + io_throttle = virXMLPropString(cur, "bps"); + if (io_throttle) { + def->blkiothrottle.bps = strtoull(io_throttle, NULL, 10); + VIR_FREE(io_throttle); + } + + io_throttle = virXMLPropString(cur, "bps_rd"); + if (io_throttle) { + def->blkiothrottle.bps_rd = strtoull(io_throttle, NULL, 10); + VIR_FREE(io_throttle); + } + + io_throttle = virXMLPropString(cur, "bps_wr"); + if (io_throttle) { + def->blkiothrottle.bps_wr = strtoull(io_throttle, NULL, 10); + VIR_FREE(io_throttle); + } + + io_throttle = virXMLPropString(cur, "iops"); + if (io_throttle) { + def->blkiothrottle.iops = strtoull(io_throttle, NULL, 10); + VIR_FREE(io_throttle); + } + + io_throttle = virXMLPropString(cur, "iops_rd"); + if (io_throttle) { + def->blkiothrottle.iops_rd = strtoull(io_throttle, NULL, 10); + VIR_FREE(io_throttle); + } + + io_throttle = virXMLPropString(cur, "iops_wr"); + if (io_throttle) { + def->blkiothrottle.iops_wr = strtoull(io_throttle, NULL, 10); + } } else if (xmlStrEqual(cur->name, BAD_CAST "readonly")) { def->readonly = 1; } else if (xmlStrEqual(cur->name, BAD_CAST "shareable")) { @@ -9249,6 +9285,47 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " <target dev='%s' bus='%s'/>\n", def->dst, bus); + /*disk I/O throttling*/ + if (def->blkiothrottle.bps + || def->blkiothrottle.bps_rd + || def->blkiothrottle.bps_wr + || def->blkiothrottle.iops + || def->blkiothrottle.iops_rd + || def->blkiothrottle.iops_wr) { + virBufferAsprintf(buf, " <iothrottle"); + if (def->blkiothrottle.bps) { + virBufferAsprintf(buf, " bps='%llu'", + def->blkiothrottle.bps); + } + + if (def->blkiothrottle.bps_rd) { + virBufferAsprintf(buf, " bps_rd='%llu'", + def->blkiothrottle.bps_rd); + } + + if (def->blkiothrottle.bps_wr) { + virBufferAsprintf(buf, " bps_wr='%llu'", + def->blkiothrottle.bps_wr); + } + + if (def->blkiothrottle.iops) { + virBufferAsprintf(buf, " iops='%llu'", + def->blkiothrottle.iops); + } + + if (def->blkiothrottle.iops_rd) { + virBufferAsprintf(buf, " iops_rd='%llu'", + def->blkiothrottle.iops_rd); + } + + if (def->blkiothrottle.iops_wr) { + virBufferAsprintf(buf, " iops_wr='%llu'", + def->blkiothrottle.iops_wr); + } + + virBufferAsprintf(buf, "/>\n"); + } + if (def->bootIndex) virBufferAsprintf(buf, " <boot order='%d'/>\n", def->bootIndex); if (def->readonly) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e07fd2f..b60a6de 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -283,6 +283,17 @@ struct _virDomainDiskDef { virDomainDiskHostDefPtr hosts; char *driverName; char *driverType; + + /*disk I/O throttling*/ + struct { + unsigned long long bps; + unsigned long long bps_rd; + unsigned long long bps_wr; + unsigned long long iops; + unsigned long long iops_rd; + unsigned long long iops_wr; + } blkiothrottle; + char *serial; int cachemode; int error_policy; /* enum virDomainDiskErrorPolicy */ diff --git a/src/driver.h b/src/driver.h index f85a1b1..741e196 100644 --- a/src/driver.h +++ b/src/driver.h @@ -725,6 +725,16 @@ typedef int (*virDrvDomainBlockPull)(virDomainPtr dom, const char *path, unsigned long bandwidth, unsigned int flags); +/* + * Block I/O throttling support + */ + +typedef int + (*virDrvDomainBlockIoThrottle)(virDomainPtr dom, + const char *disk, + virDomainBlockIoThrottleInfoPtr info, + virDomainBlockIoThrottleInfoPtr reply, + unsigned int flags); /** * _virDriver: @@ -881,6 +891,7 @@ struct _virDriver { virDrvDomainGetBlockJobInfo domainGetBlockJobInfo; virDrvDomainBlockJobSetSpeed domainBlockJobSetSpeed; virDrvDomainBlockPull domainBlockPull; + virDrvDomainBlockIoThrottle domainBlockIoThrottle; }; typedef int diff --git a/src/libvirt.c b/src/libvirt.c index 182e031..5f4514c 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -16706,3 +16706,69 @@ error: virDispatchError(dom->conn); return -1; } + +/** + * virDomainBlockIoThrottle: + * @dom: pointer to domain object + * @disk: Fully-qualified disk name + * @info: specify block I/O limits in bytes + * @reply: store block I/O limits setting + * @flags: indicate if it set or display block I/O limits info + * + * This function is mainly to enable Block I/O throttling function in libvirt. + * It is used to set or display block I/O throttling setting for specified domain. + * + * Returns 0 if the operation has started, -1 on failure. + */ +int virDomainBlockIoThrottle(virDomainPtr dom, + const char *disk, + virDomainBlockIoThrottleInfoPtr info, + virDomainBlockIoThrottleInfoPtr reply, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(dom, "disk=%p, info=%p, reply=%p,flags=%x", + disk, info, reply, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN (dom)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + conn = dom->conn; + + if (dom->conn->flags & VIR_CONNECT_RO) { + virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + if (!disk) { + virLibDomainError(VIR_ERR_INVALID_ARG, + _("disk name is NULL")); + goto error; + } + + if (!reply) { + virLibDomainError(VIR_ERR_INVALID_ARG, + _("reply is NULL")); + goto error; + } + + if (conn->driver->domainBlockIoThrottle) { + int ret; + ret = conn->driver->domainBlockIoThrottle(dom, disk, info, reply, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibDomainError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(dom->conn); + return -1; +} + diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index afea29b..0a79167 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -493,6 +493,7 @@ LIBVIRT_0.9.7 { global: virDomainReset; virDomainSnapshotGetParent; + virDomainBlockIoThrottle; } LIBVIRT_0.9.5; # .... define new API here using predicted next version number .... diff --git a/src/util/xml.h b/src/util/xml.h index d30e066..14b35b2 100644 --- a/src/util/xml.h +++ b/src/util/xml.h @@ -50,6 +50,9 @@ xmlNodePtr virXPathNode(const char *xpath, int virXPathNodeSet(const char *xpath, xmlXPathContextPtr ctxt, xmlNodePtr **list); +int virXMLStringToULongLong (const char *stringval, + unsigned long long *value); + char * virXMLPropString(xmlNodePtr node, const char *name); -- 1.7.1

5 15

[libvirt] [PATCH v2 0/2] Allow to make disk optional on migration
by Michal Privoznik 13 Oct '11

13 Oct '11

If user wants to migrate a machine, it's all or nothing. Either he/she has all storage accessible under same paths, or migration will fail. But there are some cases, where allowing migration to drop some disks, may be useful. E.g. when host gets itself to a state where domains need to be migrated so host can be say rebooted. Therefore we might want to give user possibility to mark some cdroms/floppy as optional, meaning if destination cannot access them, they get ejected (on destination). This idea is implemented via <migration> element, which basically says what to do with (currently) disk on migration. Right now it contains only one attribute 'optional' accepting values 'require', 'optional' and 'drop'. Then, if destination cannot access a path and corresponding disk is marked as: a) 'require' - migration will fail b) 'optional' - disk gets force ejected (source is free()'d), regardless tray being locked c) 'drop' - disk gets force ejected even if destination could access the path. diff to v1: -change xml representation from 'yes' & 'no' to 'require', 'optional' & 'drop' Michal Privoznik (2): migration: Introduce <migration> element for cdrom and floppy qemu: Implement migration optional disk docs/formatdomain.html.in | 23 ++++++ docs/schemas/domaincommon.rng | 17 ++++ src/conf/domain_conf.c | 78 +++++++++++++++++++- src/conf/domain_conf.h | 17 ++++ src/libvirt_private.syms | 2 + src/qemu/qemu_migration.c | 51 ++++++++++++- .../domain-qemu-complex-migration.xml | 68 +++++++++++++++++ 7 files changed, 253 insertions(+), 3 deletions(-) create mode 100644 tests/domainschemadata/domain-qemu-complex-migration.xml -- 1.7.3.4

3 5

[libvirt] [test-API][PATCH 0/5] Update migrate module
by Wayne Sun 13 Oct '11

13 Oct '11

Current migrate module include the remote_guest_define function which has problem with transient guest created on local, it can't find persistent domain xml on local and copy to remote. This series fixes aim to solve this problem. First, remote_guest_define function in migrate is removed, then add support for define domain on remote in define module. New xml element uuid is added into xmlgenerator, and new variable static_uuid is introduced into env.cfg, this will help domain created on local and defined on remote get the same uuid. At last, all related conf files are updated. [test-API][PATCH 1/5] Remove remote_guest_define function in migrate [test-API][PATCH 2/5] Add new variable static_uuid into env.cfg [test-API][PATCH 3/5] Update add_option_value function in parser [test-API][PATCH 4/5] Add new element uuid in xmlgenerator [test-API][PATCH 5/5] Update the migration case conf file .../migration/ssh_persistent_paused_with_dst.conf | 192 ------------------- .../migration/ssh_persistent_running_with_dst.conf | 192 ------------------- cases/migration/ssh_transient_paused_with_dst.conf | 200 -------------------- .../migration/ssh_transient_running_with_dst.conf | 198 ------------------- .../migration/tcp_persistent_paused_with_dst.conf | 192 ------------------- .../migration/tcp_persistent_running_with_dst.conf | 192 ------------------- .../tcp_sasl_persistent_paused_with_dst.conf | 64 ------- .../tcp_sasl_persistent_running_with_dst.conf | 64 ------- .../tcp_sasl_transient_paused_with_dst.conf | 64 ------- .../tcp_sasl_transient_running_with_dst.conf | 64 ------- cases/migration/tcp_transient_paused_with_dst.conf | 200 -------------------- .../migration/tcp_transient_running_with_dst.conf | 198 ------------------- .../migration/tls_persistent_paused_with_dst.conf | 192 ------------------- .../migration/tls_persistent_running_with_dst.conf | 192 ------------------- .../tls_sasl_persistent_paused_with_dst.conf | 64 ------- .../tls_sasl_persistent_running_with_dst.conf | 64 ------- .../tls_sasl_transient_paused_with_dst.conf | 64 ------- .../tls_sasl_transient_running_with_dst.conf | 64 ------- cases/migration/tls_transient_paused_with_dst.conf | 200 -------------------- .../migration/tls_transient_running_with_dst.conf | 198 ------------------- env.cfg | 3 - parser.py | 12 +- repos/domain/migrate.py | 31 +++- utils/Python/xmlgenerator.py | 7 - 24 files changed, 33 insertions(+), 2878 deletions(-)

2 10

[libvirt] [PATCH] [libvirt-tck] Allow to pass --timer to prove
by Guido Günther 13 Oct '11

13 Oct '11

so execution times can be reported too. --- bin/libvirt-tck | 11 ++++++++++- 1 files changed, 10 insertions(+), 1 deletions(-) diff --git a/bin/libvirt-tck b/bin/libvirt-tck index dcb053c..816234b 100644 --- a/bin/libvirt-tck +++ b/bin/libvirt-tck @@ -109,6 +109,10 @@ Generate an archive containing all the raw test results. The filename given should end in either C<.bz>, C<.tar.gz> or C<.tgz> +=item --timer + +Print elapsed time after each test. + =back =cut @@ -136,6 +140,7 @@ my $quiet = 0; my $help = 0; my $force = 0; my $debug = 0; +my $timer = 0; my $archive; my $config = catfile($confdir, "default.cfg"); my $format = "text"; @@ -149,7 +154,8 @@ if (!GetOptions("verbose" => \$verbose, "config=s" => \$config, "force" => \$force, "format=s" => \$format, - "testdir=s" => \$testdir) || $help) { + "testdir=s" => \$testdir, + "timer" => \$timer) || $help) { pod2usage(-verbose => $help, -output => $help ? \*STDOUT : \*STDERR, -exitval => $help ? 0 : 1); @@ -204,6 +210,9 @@ if ($format eq "xml") { -output => \*STDERR); } +if ($timer) { + push @newargv, "--timer"; +} # This env variable is the only way to pass config into # the Sys::Virt::TCK module from here -- 1.7.6.3

2 2

[libvirt] [libvirt-glib] Don't leak virStreams, take the reference
by Marc-André Lureau 13 Oct '11

13 Oct '11

Without this change, after virStreamNew() and g_value_dup_boxed() it will have refcnt = 2 and will leak when GVirStream is destroyed. --- libvirt-gobject/libvirt-gobject-stream.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-stream.c b/libvirt-gobject/libvirt-gobject-stream.c index 88e3a40..4aeb182 100644 --- a/libvirt-gobject/libvirt-gobject-stream.c +++ b/libvirt-gobject/libvirt-gobject-stream.c @@ -160,7 +160,7 @@ static void gvir_stream_set_property(GObject *object, case PROP_HANDLE: if (priv->handle) virStreamFree(priv->handle); - priv->handle = g_value_dup_boxed(value); + priv->handle = g_value_get_boxed(value); break; default: -- 1.7.6.2

2 1

[libvirt] RPM build failure on Redhat6
by Wayne Xia 13 Oct '11

13 Oct '11

Hi, I want to build rpm for latest libvit version on Redhat6, but when I type: ./autogen.sh --system make rpm -j4 error happens: sitemap.html.tmp index.html.tmp archdomain.html.tmp downloads.html.tmp auth.html.tmp internals/locking.html.tmp archnode.html.tmp hvsupport.html.tmp bugs.html.tmp java.html.tmp locking.html.tmp logging.html.tmp testapi.html.tmp formatnetwork.html.tmp bindings.html.tmp uri.html.tmp drvtest.html.tmp contact.html.tmp formatnwfilter.html.tmp intro.html.tmp drvxen.html.tmp drvesx.html.tmp drvqemu.html.tmp drvvmware.html.tmp testsuites.html.tmp devguide.html.tmp firewall.html.tmp architecture.html.tmp todo.html.tmp formatsnapshot.html.tmp formatstorage.html.tmp deployment.html.tmp internals.html.tmp api_extension.html.tmp drvuml.html.tmp virshcmdref.html.tmp csharp.html.tmp python.html.tmp drvremote.html.tmp archstorage.html.tmp remote.html.tmp drivers.html.tmp formatnode.html.tmp make[2]: Leaving directory `/home/xiawenc/WorkDir/Source/libvirt/libvirt/docs' make[1]: *** [distdir] Error 1 make[1]: Leaving directory `/home/xiawenc/WorkDir/Source/libvirt/libvirt' make: *** [rpm] Error 2 does anyone knows what is happening? -- Best Regards Wayne Xia mail:xiawenc@linux.vnet.ibm.com tel:86-010-82450803

3 4

[libvirt] [PATCH] qemu: Check for domain being active on successful job acquire
by Michal Privoznik 13 Oct '11

13 Oct '11

As this is needed. Although some functions check for domain being active before obtaining job, we need to check it after, because obtaining job unlocks domain object, during which a state of domain can be changed. --- src/qemu/qemu_driver.c | 75 ++++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 69 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ec01cd5..1efc66b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1948,9 +1948,18 @@ static int qemuDomainInjectNMI(virDomainPtr domain, unsigned int flags) if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; + + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto endjob; + } + qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorInjectNMI(priv->mon); qemuDomainObjExitMonitorWithDriver(driver, vm); + +endjob: if (qemuDomainObjEndJob(driver, vm) == 0) { vm = NULL; goto cleanup; @@ -4387,7 +4396,7 @@ static char *qemuDomainGetXMLDesc(virDomainPtr dom, virDomainObjPtr vm; char *ret = NULL; unsigned long balloon; - int err; + int err = 0; /* Flags checked by virDomainDefFormat */ @@ -4413,9 +4422,17 @@ static char *qemuDomainGetXMLDesc(virDomainPtr dom, if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_QUERY) < 0) goto cleanup; + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto endjob; + } + qemuDomainObjEnterMonitorWithDriver(driver, vm); err = qemuMonitorGetBalloonInfo(priv->mon, &balloon); qemuDomainObjExitMonitorWithDriver(driver, vm); + +endjob: if (qemuDomainObjEndJob(driver, vm) == 0) { vm = NULL; goto cleanup; @@ -7153,6 +7170,12 @@ qemudDomainBlockStatsFlags (virDomainPtr dom, if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) goto cleanup; + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto endjob; + } + qemuDomainObjEnterMonitor(driver, vm); tmp = *nparams; ret = qemuMonitorGetBlockStatsParamsNumber(priv->mon, nparams); @@ -8672,6 +8695,12 @@ qemuDomainMigrateSetMaxSpeed(virDomainPtr dom, if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MIGRATION_OP) < 0) goto cleanup; + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto endjob; + } + VIR_DEBUG("Setting migration bandwidth to %luMbs", bandwidth); qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorSetMigrationSpeed(priv->mon, bandwidth); @@ -8680,6 +8709,7 @@ qemuDomainMigrateSetMaxSpeed(virDomainPtr dom, if (ret == 0) priv->migMaxBandwidth = bandwidth; +endjob: if (qemuDomainObjEndJob(driver, vm) == 0) vm = NULL; } else { @@ -8774,6 +8804,12 @@ qemuDomainSnapshotCreateActive(virConnectPtr conn, if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) return -1; + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto endjob; + } + if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { /* savevm monitor command pauses the domain emitting an event which * confuses libvirt since it's not notified when qemu resumes the @@ -8823,6 +8859,7 @@ cleanup: _("resuming after snapshot failed")); } +endjob: if (vm && qemuDomainObjEndJob(driver, vm) == 0) { /* Only possible if a transient vm quit while our locks were down, * in which case we don't want to save snapshot metadata. */ @@ -9045,6 +9082,13 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn, if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) return -1; + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto endjob; + } + + if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { /* In qemu, snapshot_blkdev on a single disk will pause cpus, * but this confuses libvirt since notifications are not given @@ -9127,12 +9171,14 @@ cleanup: (persist && virDomainSaveConfig(driver->configDir, vm->newDef) < 0)) ret = -1; - if (qemuDomainObjEndJob(driver, vm) == 0) { + } + +endjob: + if (vm && (qemuDomainObjEndJob(driver, vm) == 0)) { /* Only possible if a transient vm quit while our locks were down, * in which case we don't want to save snapshot metadata. */ *vmptr = NULL; ret = -1; - } } return ret; @@ -10272,20 +10318,28 @@ static int qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd, goto cleanup; } + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain is not running")); + goto endjob; + } + priv = vm->privateData; qemuDomainObjTaint(driver, vm, VIR_DOMAIN_TAINT_CUSTOM_MONITOR, -1); hmp = !!(flags & VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP); - if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) - goto cleanup; qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorArbitraryCommand(priv->mon, cmd, result, hmp); qemuDomainObjExitMonitorWithDriver(driver, vm); + +endjob: if (qemuDomainObjEndJob(driver, vm) == 0) { vm = NULL; - goto cleanup; } cleanup: @@ -10522,10 +10576,19 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; + + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain is not running")); + goto endjob; + } + qemuDomainObjEnterMonitorWithDriver(driver, vm); priv = vm->privateData; ret = qemuMonitorBlockJob(priv->mon, device, bandwidth, info, mode); qemuDomainObjExitMonitorWithDriver(driver, vm); + +endjob: if (qemuDomainObjEndJob(driver, vm) == 0) { vm = NULL; goto cleanup; -- 1.7.3.4

2 2

[libvirt] [PATCH] events: Propose a separate lock for event queue
by Michal Privoznik 13 Oct '11

13 Oct '11

Currently, push & pop from event queue (both server & client side) rely on lock from higher levels, e.g. on driver lock (qemu), private_data (remote), ...; This alone is not sufficient as not every function that interacts with this queue can/does lock, esp. in client where we have a different approach, "passing the buck". Therefore we need a separate lock just to protect event queue. For more info see: https://bugzilla.redhat.com/show_bug.cgi?id=743817 --- src/conf/domain_event.c | 65 +++++++++++++++++++++++++++++++++++++++++------ src/conf/domain_event.h | 1 + 2 files changed, 58 insertions(+), 8 deletions(-) diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 3189346..6cc8168 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -547,6 +547,24 @@ virDomainEventQueuePtr virDomainEventQueueNew(void) return ret; } +static void +virDomainEventStateLock(virDomainEventStatePtr state) +{ + if (!state) + return; + + virMutexLock(&state->lock); +} + +static void +virDomainEventStateUnlock(virDomainEventStatePtr state) +{ + if (!state) + return; + + virMutexUnlock(&state->lock); +} + /** * virDomainEventStateFree: * @list: virDomainEventStatePtr to free @@ -564,6 +582,8 @@ virDomainEventStateFree(virDomainEventStatePtr state) if (state->timer != -1) virEventRemoveTimeout(state->timer); + + virMutexDestroy(&state->lock); VIR_FREE(state); } @@ -590,6 +610,13 @@ virDomainEventStateNew(virEventTimeoutCallback timeout_cb, goto error; } + if (virMutexInit(&state->lock) < 0) { + virReportSystemError(errno, "%s", + _("unable to initialize state mutex")); + VIR_FREE(state); + goto error; + } + if (VIR_ALLOC(state->callbacks) < 0) { virReportOOMError(); goto error; @@ -1161,18 +1188,26 @@ void virDomainEventStateQueue(virDomainEventStatePtr state, virDomainEventPtr event) { + int count; + if (state->timer < 0) { virDomainEventFree(event); return; } + virDomainEventStateLock(state); + if (virDomainEventQueuePush(state->queue, event) < 0) { VIR_DEBUG("Error adding event to queue"); virDomainEventFree(event); } - if (state->queue->count == 1) + count = state->queue->count; + virDomainEventStateUnlock(state); + + if (count == 1) virEventUpdateTimeout(state->timer, 0); + } void @@ -1182,6 +1217,7 @@ virDomainEventStateFlush(virDomainEventStatePtr state, { virDomainEventQueue tempQueue; + virDomainEventStateLock(state); state->isDispatching = true; /* Copy the queue, so we're reentrant safe when dispatchFunc drops the @@ -1190,6 +1226,7 @@ virDomainEventStateFlush(virDomainEventStatePtr state, tempQueue.events = state->queue->events; state->queue->count = 0; state->queue->events = NULL; + virDomainEventStateUnlock(state); virEventUpdateTimeout(state->timer, -1); virDomainEventQueueDispatch(&tempQueue, @@ -1198,9 +1235,11 @@ virDomainEventStateFlush(virDomainEventStatePtr state, opaque); /* Purge any deleted callbacks */ + virDomainEventStateLock(state); virDomainEventCallbackListPurgeMarked(state->callbacks); state->isDispatching = false; + virDomainEventStateUnlock(state); } int @@ -1208,11 +1247,16 @@ virDomainEventStateDeregister(virConnectPtr conn, virDomainEventStatePtr state, virConnectDomainEventCallback callback) { + int ret; + + virDomainEventStateLock(state); if (state->isDispatching) - return virDomainEventCallbackListMarkDelete(conn, - state->callbacks, callback); + ret = virDomainEventCallbackListMarkDelete(conn, + state->callbacks, callback); else - return virDomainEventCallbackListRemove(conn, state->callbacks, callback); + ret = virDomainEventCallbackListRemove(conn, state->callbacks, callback); + virDomainEventStateUnlock(state); + return ret; } int @@ -1220,10 +1264,15 @@ virDomainEventStateDeregisterAny(virConnectPtr conn, virDomainEventStatePtr state, int callbackID) { + int ret; + + virDomainEventStateLock(state); if (state->isDispatching) - return virDomainEventCallbackListMarkDeleteID(conn, - state->callbacks, callbackID); + ret = virDomainEventCallbackListMarkDeleteID(conn, + state->callbacks, callbackID); else - return virDomainEventCallbackListRemoveID(conn, - state->callbacks, callbackID); + ret = virDomainEventCallbackListRemoveID(conn, + state->callbacks, callbackID); + virDomainEventStateUnlock(state); + return ret; } diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h index b06be16..08930ed 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -61,6 +61,7 @@ struct _virDomainEventState { int timer; /* Flag if we're in process of dispatching */ bool isDispatching; + virMutex lock; }; typedef struct _virDomainEventState virDomainEventState; typedef virDomainEventState *virDomainEventStatePtr; -- 1.7.3.4

3 5

[libvirt] [PATCH v4 0/2] Allow domain reboot after core dump
by Michal Privoznik 13 Oct '11

13 Oct '11

This is the 4th (and hopefully last) version of these patches. diff to v3: -split in 2 patches -and some other Eric's suggestions taken in Michal Privoznik (2): virDomainCoreDump: Introduce VIR_DUMP_RESET flag qemu: Implement VIR_DUMP_RESET include/libvirt/libvirt.h.in | 1 + src/libvirt.c | 15 ++++++++++++++- src/qemu/qemu_driver.c | 9 ++++++++- tools/virsh.c | 3 +++ tools/virsh.pod | 4 +++- 5 files changed, 29 insertions(+), 3 deletions(-) -- 1.7.3.4

2 8

[libvirt] Possible security hole? unprivileged user can use virsh to overwrite sensitive system file
by Hong Xiang 13 Oct '11

13 Oct '11

I found there's a way for a unprivileged user to overwrite sensitive system file with virsh, here's how: 1. (as an unprivileged user) start virsh and connect to the r/w socket of libvirtd: virsh -c qemu+unix:///system?socket=/var/run/libvirt/libvirt-sock 2. start a guest, then issue 'save' or 'dump' command, giving a sensitive system file path as the <file> parameter, for example, '/etc/passwd'; 3. the sensitive system file will be overwritten; Attached is a test log. I'm using libvirt-0.8.7 on a OpenClient for RHEL 6.1. And latest libvirt code shows the same symptom. BTW, virsh expands the <file> parameter in step to an absolute path if user-provided is not, and libvirtd interprets it as a local file. IMHO it does not look quite right, especially when the virsh-to-libvirtd connection is remote. -- Thanks. Hong Xiang

3 3