Re: [libvirt] [PATCH 11/10] migrate: detect xml incompatibility
On Thu, Jun 02, 2011 at 05:42:27PM -0600, Eric Blake wrote:
> Detected by Coverity. Bug introduced in 08106e2044 (unreleased).
>
> * src/conf/domain_conf.c (virDomainChannelDefCheckABIStability):
> Use correct sizeof operand.
> ---
> src/conf/domain_conf.c |3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 9975bca..0d9fef4 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -7269,7 +7269,8 @@ static bool
> virDomainChannelDefCheckABIStability(virDomainChrDefPtr src,
> }
> break;
> case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD:
> -if (memcmp(src->target.addr, dst->target.addr,
> sizeof(src->target.addr)) != 0) {
> +if (memcmp(src->target.addr, dst->target.addr,
> + sizeof(*src->target.addr)) != 0) {
> char *saddr = virSocketFormatAddrFull(src->target.addr, true,
> ":");
> char *daddr = virSocketFormatAddrFull(dst->target.addr, true,
> ":");
> virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
Oops, good catch !!!
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 10/10] build: silence coverity false positive
On Thu, Jun 02, 2011 at 05:24:25PM -0600, Eric Blake wrote: > Coverity couldn't see that priv is clean on failure. But on failure, > we might as well guarantee that callers don't try to free uninitialized > memory. > > * src/remote/remote_driver.c (remoteGenericOpen): Even on failure, > pass priv back to caller. > --- > > In reading my own mail, I decided that patch 10 can be made simpler. > > v2: rewrite to avoid sa_assert and be more robust. > > src/remote/remote_driver.c |6 ++ > 1 files changed, 2 insertions(+), 4 deletions(-) > > diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c > index 14c3d24..8335a1a 100644 > --- a/src/remote/remote_driver.c > +++ b/src/remote/remote_driver.c > @@ -2545,10 +2545,8 @@ remoteGenericOpen(virConnectPtr conn, > virConnectAuthPtr auth, > * use the UNIX transport. This handles Xen driver > * which doesn't have its own impl of the network APIs. */ > struct private_data *priv; > -int ret; > -ret = remoteOpenSecondaryDriver(conn, auth, flags, &priv); > -if (ret == VIR_DRV_OPEN_SUCCESS) > -*genericPrivateData = priv; > +int ret = remoteOpenSecondaryDriver(conn, auth, flags, &priv); > +*genericPrivateData = priv; > return ret; > } > } I really prefer the second version, ACK Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 09/10] build: silence coverity false positive
On Thu, Jun 02, 2011 at 05:08:01PM -0600, Eric Blake wrote:
> Coverity complained that infd could be -1 at the point where it is
> passed to write, when in reality, this code can only be reached if
> infd is non-negative.
>
> * src/util/command.c (virCommandProcessIO): Help out coverity.
> ---
> src/util/command.c |3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/src/util/command.c b/src/util/command.c
> index a2f7ff6..b51bdcf 100644
> --- a/src/util/command.c
> +++ b/src/util/command.c
> @@ -961,6 +961,9 @@ virCommandProcessIO(virCommandPtr cmd)
> } else {
> int done;
>
> +/* Coverity 5.3.0 can't see that we only get here if
> + * infd is in the set because it was non-negative. */
> +sa_assert(infd != -1);
> done = write(infd, cmd->inbuf + inoff,
> inlen - inoff);
> if (done < 0) {
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/3] update qemuDomainGetBlkioParameters to use flags
---
src/qemu/qemu_driver.c | 124 +---
1 files changed, 86 insertions(+), 38 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 43bb0fc..7b855d6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4773,18 +4773,16 @@ static int qemuDomainGetBlkioParameters(virDomainPtr
dom,
int i;
virCgroupPtr group = NULL;
virDomainObjPtr vm = NULL;
+virDomainDefPtr persistentDef = NULL;
unsigned int val;
int ret = -1;
int rc;
+bool isActive;
-virCheckFlags(0, -1);
+virCheckFlags(VIR_DOMAIN_PARAM_LIVE |
+ VIR_DOMAIN_PARAM_CONFIG, -1);
qemuDriverLock(driver);
-if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) {
-qemuReportError(VIR_ERR_NO_SUPPORT, _("blkio cgroup isn't mounted"));
-goto cleanup;
-}
-
vm = virDomainFindByUUID(&driver->domains, dom->uuid);
if (vm == NULL) {
@@ -4793,12 +4791,6 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom,
goto cleanup;
}
-if (!virDomainObjIsActive(vm)) {
-qemuReportError(VIR_ERR_OPERATION_INVALID,
-"%s", _("domain is not running"));
-goto cleanup;
-}
-
if ((*nparams) == 0) {
/* Current number of blkio parameters supported by cgroups */
*nparams = QEMU_NB_BLKIO_PARAM;
@@ -4812,37 +4804,93 @@ static int qemuDomainGetBlkioParameters(virDomainPtr
dom,
goto cleanup;
}
-if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) {
-qemuReportError(VIR_ERR_INTERNAL_ERROR,
-_("cannot find cgroup for domain %s"), vm->def->name);
-goto cleanup;
+isActive = virDomainObjIsActive(vm);
+
+if (flags == VIR_DOMAIN_PARAM_CURRENT) {
+if (isActive)
+flags = VIR_DOMAIN_PARAM_LIVE;
+else
+flags = VIR_DOMAIN_PARAM_CONFIG;
}
-for (i = 0; i < *nparams; i++) {
-virTypedParameterPtr param = ¶ms[i];
-val = 0;
-param->value.ui = 0;
-param->type = VIR_TYPED_PARAM_UINT;
+if (flags & VIR_DOMAIN_PARAM_LIVE) {
+if (!isActive) {
+qemuReportError(VIR_ERR_OPERATION_INVALID,
+"%s", _("domain is not running"));
+goto cleanup;
+}
-switch (i) {
-case 0: /* fill blkio weight here */
-rc = virCgroupGetBlkioWeight(group, &val);
-if (rc != 0) {
-virReportSystemError(-rc, "%s",
- _("unable to get blkio weight"));
-goto cleanup;
-}
-if (virStrcpyStatic(param->field, VIR_DOMAIN_BLKIO_WEIGHT) ==
NULL) {
-qemuReportError(VIR_ERR_INTERNAL_ERROR,
-"%s", _("Field blkio weight too long for
destination"));
-goto cleanup;
+if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) {
+qemuReportError(VIR_ERR_NO_SUPPORT, _("blkio cgroup isn't
mounted"));
+goto cleanup;
+}
+
+if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0)
{
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+_("cannot find cgroup for domain %s"),
vm->def->name);
+goto cleanup;
+}
+}
+
+if (flags & VIR_DOMAIN_PARAM_CONFIG) {
+if (!vm->persistent) {
+qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
+_("cannot change persistent config of a transient
domain"));
+goto cleanup;
+}
+if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm)))
+goto cleanup;
+}
+
+if (flags & VIR_DOMAIN_PARAM_LIVE) {
+for (i = 0; i < *nparams; i++) {
+virTypedParameterPtr param = ¶ms[i];
+val = 0;
+param->value.ui = 0;
+param->type = VIR_TYPED_PARAM_UINT;
+
+switch (i) {
+case 0: /* fill blkio weight here */
+rc = virCgroupGetBlkioWeight(group, &val);
+if (rc != 0) {
+virReportSystemError(-rc, "%s",
+ _("unable to get blkio weight"));
+goto cleanup;
+}
+if (virStrcpyStatic(param->field, VIR_DOMAIN_BLKIO_WEIGHT) ==
NULL) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+"%s", _("Field blkio weight too long for
destination"));
+goto cleanup;
+}
+param->value.ui = val;
+break;
+
+default:
+break;
+/* should not hit here */
}
-param->value.ui = val;
-break;
+}
+} else if (flags & VIR_DOMAIN_
[libvirt] [PATCH v2 1/3] unify PARAM_CURRENT/PARAM_LIVE/PARAM_CONFIG flags
This patch unify the PARAM_CURRENT/PARAM_LIVE/PARAM_CONFIG flags as
Daniel Veillard suggested.
---
include/libvirt/libvirt.h.in | 58 -
src/esx/esx_driver.c | 10 +-
src/libvirt.c|4 +-
src/openvz/openvz_driver.c | 10 +-
src/qemu/qemu_driver.c | 194 +-
src/test/test_driver.c | 42 +-
src/uml/uml_driver.c |4 +-
src/vbox/vbox_tmpl.c | 22 +++---
tools/virsh.c| 74
9 files changed, 196 insertions(+), 222 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 8058229..8ef07b0 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -142,6 +142,22 @@ typedef enum {
} virDomainCrashedReason;
/**
+ * virDomainParamFlags:
+ *
+ * common flags for commands that support --current, --live, --config
+ * and --force parameters
+ */
+
+typedef enum {
+VIR_DOMAIN_PARAM_CURRENT = 0, /* affect current domain state
*/
+VIR_DOMAIN_PARAM_LIVE= (1 << 0), /* affect active domain */
+VIR_DOMAIN_PARAM_CONFIG = (1 << 1), /* affect next boot */
+VIR_DOMAIN_PARAM_FORCE = (1 << 2), /* Forcibly modify device
+ (ex. force eject a cdrom) */
+VIR_DOMAIN_PARAM_MAXIMUM = (1 << 3), /* affect Max rather than
current */
+} virDomainParamFlags;
+
+/**
* virDomainInfoPtr:
*
* a virDomainInfo is a structure filled by virDomainGetInfo() and extracting
@@ -338,12 +354,6 @@ typedef virTypedParameter *virTypedParameterPtr;
/* Management of scheduler parameters */
-typedef enum {
-VIR_DOMAIN_SCHEDPARAM_CURRENT = 0,/* affect current domain state */
-VIR_DOMAIN_SCHEDPARAM_LIVE= (1 << 0), /* Affect active domain */
-VIR_DOMAIN_SCHEDPARAM_CONFIG = (1 << 1), /* Affect next boot */
-} virDomainSchedParameterFlags;
-
/*
* Fetch scheduler parameters, caller allocates 'params' field of size
'nparams'
*/
@@ -799,13 +809,6 @@ int virDomainGetBlkioParameters(virDomainPtr domain,
/* Manage memory parameters. */
-/* flags for setting memory parameters */
-typedef enum {
-VIR_DOMAIN_MEMORY_PARAM_CURRENT = 0,/* affect current domain state
*/
-VIR_DOMAIN_MEMORY_PARAM_LIVE= (1 << 0), /* affect active domain */
-VIR_DOMAIN_MEMORY_PARAM_CONFIG = (1 << 1) /* affect next boot */
-} virMemoryParamFlags;
-
/**
* VIR_DOMAIN_MEMORY_PARAM_UNLIMITED:
*
@@ -859,15 +862,6 @@ int virDomainGetMemoryParameters(virDomainPtr domain,
virTypedParameterPtr params,
int *nparams, unsigned int flags);
-/* Memory size modification flags. */
-typedef enum {
-VIR_DOMAIN_MEM_CURRENT = 0,/* affect current domain state */
-VIR_DOMAIN_MEM_LIVE= (1 << 0), /* affect active domain */
-VIR_DOMAIN_MEM_CONFIG = (1 << 1), /* affect next boot */
-VIR_DOMAIN_MEM_MAXIMUM = (1 << 2), /* affect Max rather than current */
-} virDomainMemoryModFlags;
-
-
/*
* Dynamic control of domains
*/
@@ -1023,16 +1017,6 @@ struct _virVcpuInfo {
};
typedef virVcpuInfo *virVcpuInfoPtr;
-/* Flags for controlling virtual CPU hot-plugging. */
-typedef enum {
-/* Must choose at least one of these two bits; SetVcpus can choose both */
-VIR_DOMAIN_VCPU_LIVE= (1 << 0), /* Affect active domain */
-VIR_DOMAIN_VCPU_CONFIG = (1 << 1), /* Affect next boot */
-
-/* Additional flags to be bit-wise OR'd in */
-VIR_DOMAIN_VCPU_MAXIMUM = (1 << 2), /* Max rather than current count */
-} virDomainVcpuFlags;
-
int virDomainSetVcpus (virDomainPtr domain,
unsigned int nvcpus);
int virDomainSetVcpusFlags (virDomainPtr domain,
@@ -1131,16 +1115,6 @@ int virDomainGetVcpus
(virDomainPtr domain,
*/
#define VIR_GET_CPUMAP(cpumaps,maplen,vcpu) &(cpumaps[(vcpu)*(maplen)])
-
-typedef enum {
-
- VIR_DOMAIN_DEVICE_MODIFY_CURRENT = 0, /* Modify device allocation based on
current domain state */
- VIR_DOMAIN_DEVICE_MODIFY_LIVE = (1 << 0), /* Modify live device allocation
*/
- VIR_DOMAIN_DEVICE_MODIFY_CONFIG = (1 << 1), /* Modify persisted device
allocation */
- VIR_DOMAIN_DEVICE_MODIFY_FORCE = (1 << 2), /* Forcibly modify device
- (ex. force eject a cdrom) */
-} virDomainDeviceModifyFlags;
-
int virDomainAttachDevice(virDomainPtr domain, const char *xml);
int virDomainDetachDevice(virDomainPtr domain, const char *xml);
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 500dc52..ffce508 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -2540,7 +2540,7 @@ esxDomainSetVcpusFlags(virDomainPtr domain, unsigned int
nvcpus,
esxVI_TaskInfoState taskInfoState;
char *t
[libvirt] [PATCH v2 0/3] add support for changing blkio parameters for inactive domains
This series enables user to change blkio parameters for inactive domains from virsh command line. CHANGES: v1-v2: - unify PARAM_CONFIG/PARAM_LIVE/PARAM_CURRENT Hu Tao (3): unify PARAM_CURRENT/PARAM_LIVE/PARAM_CONFIG flags update qemuDomainGetBlkioParameters to use flags Update qemuDomainSetBlkioParameters to use flags include/libvirt/libvirt.h.in | 58 ++ src/esx/esx_driver.c | 10 +- src/libvirt.c|4 +- src/openvz/openvz_driver.c | 10 +- src/qemu/qemu_driver.c | 442 ++ src/test/test_driver.c | 42 ++-- src/uml/uml_driver.c |4 +- src/vbox/vbox_tmpl.c | 22 +- tools/virsh.c| 74 9 files changed, 371 insertions(+), 295 deletions(-) -- 1.7.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/3] Update qemuDomainSetBlkioParameters to use flags
---
src/qemu/qemu_driver.c | 126 ++--
1 files changed, 90 insertions(+), 36 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7b855d6..edefb5a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4694,14 +4694,13 @@ static int qemuDomainSetBlkioParameters(virDomainPtr
dom,
int i;
virCgroupPtr group = NULL;
virDomainObjPtr vm = NULL;
+virDomainDefPtr persistentDef = NULL;
int ret = -1;
+bool isActive;
-virCheckFlags(0, -1);
+virCheckFlags(VIR_DOMAIN_PARAM_LIVE |
+ VIR_DOMAIN_PARAM_CONFIG, -1);
qemuDriverLock(driver);
-if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) {
-qemuReportError(VIR_ERR_NO_SUPPORT, _("blkio cgroup isn't mounted"));
-goto cleanup;
-}
vm = virDomainFindByUUID(&driver->domains, dom->uuid);
@@ -4711,49 +4710,104 @@ static int qemuDomainSetBlkioParameters(virDomainPtr
dom,
goto cleanup;
}
-if (!virDomainObjIsActive(vm)) {
-qemuReportError(VIR_ERR_OPERATION_INVALID,
-"%s", _("domain is not running"));
-goto cleanup;
+isActive = virDomainObjIsActive(vm);
+
+if (flags == VIR_DOMAIN_PARAM_CURRENT) {
+if (isActive)
+flags = VIR_DOMAIN_PARAM_LIVE;
+else
+flags = VIR_DOMAIN_PARAM_CONFIG;
}
-if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) {
-qemuReportError(VIR_ERR_INTERNAL_ERROR,
-_("cannot find cgroup for domain %s"), vm->def->name);
-goto cleanup;
+if (flags & VIR_DOMAIN_PARAM_LIVE) {
+if (!isActive) {
+qemuReportError(VIR_ERR_OPERATION_INVALID,
+"%s", _("domain is not running"));
+goto cleanup;
+}
+
+if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) {
+qemuReportError(VIR_ERR_NO_SUPPORT, _("blkio cgroup isn't
mounted"));
+goto cleanup;
+}
+
+if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0)
{
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+_("cannot find cgroup for domain %s"),
vm->def->name);
+goto cleanup;
+}
+}
+
+if (flags & VIR_DOMAIN_PARAM_CONFIG) {
+if (!vm->persistent) {
+qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
+_("cannot change persistent config of a transient
domain"));
+goto cleanup;
+}
+if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm)))
+goto cleanup;
}
ret = 0;
-for (i = 0; i < nparams; i++) {
-virTypedParameterPtr param = ¶ms[i];
+if (flags & VIR_DOMAIN_PARAM_LIVE) {
+for (i = 0; i < nparams; i++) {
+virTypedParameterPtr param = ¶ms[i];
-if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT)) {
-int rc;
-if (param->type != VIR_TYPED_PARAM_UINT) {
-qemuReportError(VIR_ERR_INVALID_ARG, "%s",
-_("invalid type for blkio weight tunable,
expected a 'unsigned int'"));
-ret = -1;
-continue;
-}
+if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT)) {
+int rc;
+if (param->type != VIR_TYPED_PARAM_UINT) {
+qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+_("invalid type for blkio weight tunable,
expected a 'unsigned int'"));
+ret = -1;
+continue;
+}
-if (params[i].value.ui > 1000 || params[i].value.ui < 100) {
-qemuReportError(VIR_ERR_INVALID_ARG, "%s",
-_("out of blkio weight range."));
+if (params[i].value.ui > 1000 || params[i].value.ui < 100) {
+qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+_("out of blkio weight range."));
+ret = -1;
+continue;
+}
+
+rc = virCgroupSetBlkioWeight(group, params[i].value.ui);
+if (rc != 0) {
+virReportSystemError(-rc, "%s",
+ _("unable to set blkio weight
tunable"));
+ret = -1;
+}
+} else {
+qemuReportError(VIR_ERR_INVALID_ARG,
+_("Parameter `%s' not supported"),
param->field);
ret = -1;
-continue;
}
+}
+} else if (flags & VIR_DOMAIN_PARAM_CONFIG) {
+for (i = 0; i < nparams; i++) {
+virTypedParameterPtr param = ¶ms[i];
-rc = virCgroupSetBlkioWeight(group, params[
Re: [libvirt] [PATCH 08/10] event: avoid memory leak on cleanup
On Thu, Jun 02, 2011 at 05:08:00PM -0600, Eric Blake wrote: > Detected by Coverity. Introduced in commit aaf2b70, and turned into > a regression in the next few commits through 4e6e6672 (unreleased). > > * src/conf/domain_event.c (virDomainEventStateFree): Free object, > per documentation. > --- > src/conf/domain_event.c |3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c > index 34a9d91..fabc1a5 100644 > --- a/src/conf/domain_event.c > +++ b/src/conf/domain_event.c > @@ -1,7 +1,7 @@ > /* > * domain_event.c: domain event queue processing helpers > * > - * Copyright (C) 2010 Red Hat, Inc. > + * Copyright (C) 2010-2011 Red Hat, Inc. > * Copyright (C) 2008 VirtualIron > * > * This library is free software; you can redistribute it and/or > @@ -555,6 +555,7 @@ virDomainEventStateFree(virDomainEventStatePtr state) > > if (state->timer != -1) > virEventRemoveTimeout(state->timer); > +VIR_FREE(state); > } ACK, otherwise we leak in various ways from different drivers, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 07/10] qemu: avoid memory leak on vcpupin
On Thu, Jun 02, 2011 at 05:07:59PM -0600, Eric Blake wrote:
> Detected by Coverity. This leaked a cpumap on every iteration
> of the loop. Leak introduced in commit 1cc4d02 (v0.9.0).
>
> * src/qemu/qemu_process.c (qemuProcessSetVcpuAffinites): Plug
> leak, and hoist allocation outside loop.
> ---
> src/qemu/qemu_process.c | 21 +
> 1 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 116253e..f175d50 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -1195,6 +1195,8 @@ qemuProcessSetVcpuAffinites(virConnectPtr conn,
> pid_t vcpupid;
> unsigned char *cpumask;
> int vcpu, cpumaplen, hostcpus, maxcpu;
> +unsigned char *cpumap = NULL;
> +int ret = -1;
>
> if (virNodeGetInfo(conn, &nodeinfo) != 0) {
> return -1;
> @@ -1216,18 +1218,18 @@ qemuProcessSetVcpuAffinites(virConnectPtr conn,
> if (maxcpu > hostcpus)
> maxcpu = hostcpus;
>
> +if (VIR_ALLOC_N(cpumap, cpumaplen) < 0) {
> +virReportOOMError();
> +return -1;
> +}
> +
> for (vcpu = 0; vcpu < def->cputune.nvcpupin; vcpu++) {
> if (vcpu != def->cputune.vcpupin[vcpu]->vcpuid)
> continue;
>
> int i;
> -unsigned char *cpumap = NULL;
> -
> -if (VIR_ALLOC_N(cpumap, cpumaplen) < 0) {
> -virReportOOMError();
> -return -1;
> -}
>
> +memset(cpumap, 0, cpumaplen);
> cpumask = (unsigned char *)def->cputune.vcpupin[vcpu]->cpumask;
> vcpupid = priv->vcpupids[vcpu];
>
> @@ -1249,11 +1251,14 @@ qemuProcessSetVcpuAffinites(virConnectPtr conn,
>cpumap,
>cpumaplen,
>maxcpu) < 0) {
> -return -1;
> +goto cleanup;
> }
> }
>
> -return 0;
> +ret = 0;
> +cleanup:
> +VIR_FREE(cpumap);
> +return ret;
> }
Whoops !!! ACK, better to allocate out of the loop, fix looks fine,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/10] remote: avoid leak on failure
On Thu, Jun 02, 2011 at 05:07:58PM -0600, Eric Blake wrote:
> Detected by Coverity. Only possible in OOM situations.
>
> * daemon/remote.c (remoteDispatchDomainScreenshot): Plug leak.
> ---
> daemon/remote.c |1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/daemon/remote.c b/daemon/remote.c
> index 2a32ee8..49058f2 100644
> --- a/daemon/remote.c
> +++ b/daemon/remote.c
> @@ -1453,6 +1453,7 @@ remoteDispatchDomainScreenshot(struct qemud_server
> *server ATTRIBUTE_UNUSED,
> *mime_p = strdup(mime);
> if (*mime_p == NULL) {
> virReportOOMError();
> +VIR_FREE(mime_p);
> goto cleanup;
> }
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 05/10] lock: avoid leak on failure
On Thu, Jun 02, 2011 at 05:07:57PM -0600, Eric Blake wrote:
> Detected by Coverity. Only possible on OOM situations.
>
> * src/locking/lock_manager.c (virLockManagerPluginNew): Plug leak.
> ---
> src/locking/lock_manager.c |3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/src/locking/lock_manager.c b/src/locking/lock_manager.c
> index 6197fd4..138cc91 100644
> --- a/src/locking/lock_manager.c
> +++ b/src/locking/lock_manager.c
> @@ -120,7 +120,7 @@ virLockManagerPluginPtr virLockManagerPluginNew(const
> char *name,
> {
> void *handle = NULL;
> virLockDriverPtr driver;
> -virLockManagerPluginPtr plugin;
> +virLockManagerPluginPtr plugin = NULL;
> const char *moddir = getenv("LIBVIRT_LOCK_MANAGER_PLUGIN_DIR");
> char *modfile = NULL;
>
> @@ -182,6 +182,7 @@ virLockManagerPluginPtr virLockManagerPluginNew(const
> char *name,
> return plugin;
>
> cleanup:
> +VIR_FREE(plugin);
> VIR_FREE(modfile);
> if (handle)
> dlclose(handle);
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 04/10] storage: avoid memory leak on stat failure
On Thu, Jun 02, 2011 at 05:07:56PM -0600, Eric Blake wrote:
> Spotted by coverity. Triggers on failed stat, although I'm not sure
> how easy that condition is, so I'm not sure if this is a runtime
> memory hog. Regression introduced in commit 8077d64 (unreleased).
>
> * src/util/storage_file.c (virStorageFileGetMetadataFromFD):
> Reduce need for malloc, avoiding a leak.
> ---
> src/util/storage_file.c | 15 +++
> 1 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/src/util/storage_file.c b/src/util/storage_file.c
> index 8dbd933..6b3b756 100644
> --- a/src/util/storage_file.c
> +++ b/src/util/storage_file.c
> @@ -831,11 +831,6 @@ virStorageFileGetMetadataFromFD(const char *path,
> int ret = -1;
> struct stat sb;
>
> -if (VIR_ALLOC_N(head, len) < 0) {
> -virReportOOMError();
> -return -1;
> -}
> -
> memset(meta, 0, sizeof (*meta));
>
> if (fstat(fd, &sb) < 0) {
> @@ -847,13 +842,17 @@ virStorageFileGetMetadataFromFD(const char *path,
>
> /* No header to probe for directories */
> if (S_ISDIR(sb.st_mode)) {
> -ret = 0;
> -goto cleanup;
> +return 0;
> }
>
> if (lseek(fd, 0, SEEK_SET) == (off_t)-1) {
> virReportSystemError(errno, _("cannot seek to start of '%s'"), path);
> -goto cleanup;
> +return -1;
> +}
> +
> +if (VIR_ALLOC_N(head, len) < 0) {
> +virReportOOMError();
> +return -1;
> }
>
> if ((len = read(fd, head, len)) < 0) {
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/10] storage: avoid memory leak
On Thu, Jun 02, 2011 at 05:07:55PM -0600, Eric Blake wrote:
> Coverity detected that options was being set by strdup but never
> freed. But why even bother with an options variable? The options
> parameter never changes! Leak present since commit 44948f5b (0.7.0).
>
> This function could probably be rewritten to take better advantage
> of virCommand, but that is more invasive.
>
> * src/storage/storage_backend_fs.c
> (virStorageBackendFileSystemMount): Avoid wasted strdup, and
> guarantee proper cleanup on all paths.
> ---
> src/storage/storage_backend_fs.c | 15 +--
> 1 files changed, 1 insertions(+), 14 deletions(-)
>
> diff --git a/src/storage/storage_backend_fs.c
> b/src/storage/storage_backend_fs.c
> index 3f4d978..207669a 100644
> --- a/src/storage/storage_backend_fs.c
> +++ b/src/storage/storage_backend_fs.c
> @@ -317,7 +317,6 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr
> pool) {
> static int
> virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) {
> char *src;
> -char *options = NULL;
> const char **mntargv;
>
> /* 'mount -t auto' doesn't seem to auto determine nfs (or cifs),
> @@ -328,7 +327,6 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr
> pool) {
> int glusterfs = (pool->def->type == VIR_STORAGE_POOL_NETFS &&
> pool->def->source.format ==
> VIR_STORAGE_POOL_NETFS_GLUSTERFS);
>
> -int option_index;
> int source_index;
>
> const char *netfs_auto_argv[] = {
> @@ -358,7 +356,7 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr
> pool) {
>
> virStoragePoolFormatFileSystemNetTypeToString(pool->def->source.format),
> NULL,
> "-o",
> -NULL,
> +"direct-io-mode=1",
> pool->def->target.path,
> NULL,
> };
> @@ -369,7 +367,6 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr
> pool) {
> } else if (glusterfs) {
> mntargv = glusterfs_argv;
> source_index = 3;
> -option_index = 5;
> } else {
> mntargv = fs_argv;
> source_index = 3;
> @@ -405,12 +402,6 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr
> pool) {
> }
>
> if (pool->def->type == VIR_STORAGE_POOL_NETFS) {
> -if (pool->def->source.format == VIR_STORAGE_POOL_NETFS_GLUSTERFS) {
> -if ((options = strdup("direct-io-mode=1")) == NULL) {
> -virReportOOMError();
> -return -1;
> -}
> -}
> if (virAsprintf(&src, "%s:%s",
> pool->def->source.host.name,
> pool->def->source.dir) == -1) {
> @@ -426,10 +417,6 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr
> pool) {
> }
> mntargv[source_index] = src;
>
> -if (glusterfs) {
> -mntargv[option_index] = options;
> -}
> -
> if (virRun(mntargv, NULL) < 0) {
> VIR_FREE(src);
> return -1;
Okay it seems we unconditionally set this option for glusterfs, but
I'm still confused by the initial author intent there. It would be good
if someone else could double check, this looks good but I'm not 100%
sure and glusterfs is not used that often.
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 02/10] libvirtd: avoid leak on failure
On Thu, Jun 02, 2011 at 05:07:54PM -0600, Eric Blake wrote:
> Spotted by Coverity. Only possible on an OOM condition, so
> unlikely to bite in the wild.
>
> * daemon/libvirtd.c (qemudSetLogging): Don't leak memory.
> ---
> daemon/libvirtd.c |4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> index aec81cf..728031f 100644
> --- a/daemon/libvirtd.c
> +++ b/daemon/libvirtd.c
> @@ -2779,8 +2779,10 @@ qemudSetLogging(struct qemud_server *server,
> virConfPtr conf,
> goto free_and_fail;
>
> if (virAsprintf(&tmp, "%d:file:%s/.libvirt/libvirtd.log",
> -virLogGetDefaultPriority(), userdir) == -1)
> +virLogGetDefaultPriority(), userdir) == -1) {
> +VIR_FREE(userdir);
> goto out_of_memory;
> +}
> }
> } else {
> if (virAsprintf(&tmp, "%d:stderr", virLogGetDefaultPriority()) <
> 0)
ACK, userdir is defined only on the local scope, so that's the best way
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/10] command: avoid leak on failure
On Thu, Jun 02, 2011 at 05:07:53PM -0600, Eric Blake wrote: > Detected by Coverity. While it is possible on OOM condition, as > well as with bad code that passes binary == NULL, it is unlikely > to be encountered in the wild. > > * src/util/command.c (virCommandNewArgList): Don't leak memory. > --- > src/util/command.c |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/src/util/command.c b/src/util/command.c > index 288958e..a2f7ff6 100644 > --- a/src/util/command.c > +++ b/src/util/command.c > @@ -142,7 +142,7 @@ virCommandNewArgList(const char *binary, ...) > const char *arg; > > if (!cmd || cmd->has_error) > -return NULL; > +return cmd; > > va_start(list, binary); > while ((arg = va_arg(list, const char *)) != NULL) ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix minor issues in libxenlight managed save
On Thu, Jun 02, 2011 at 04:04:40PM -0600, Jim Fehlig wrote: > There were a few minor issues in commit 5b6c961e > - leak managed save path > - leak managed save fd > - functions that open an fd should also close it Oops, thanks for quickly checking this out ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Building on Solaris 11 Express
On Mon, May 30, 2011 at 08:52, Matthias Bolte
wrote:
> 2011/5/30 Ruben Kerkhof :
>> On Sun, May 29, 2011 at 19:45, Matthias Bolte
>> wrote:
>>> 2011/5/29 Richard Laager :
On Sun, 2011-05-29 at 12:34 +0200, Matthias Bolte wrote:
> > So I tried building libvirt on Solaris 11 Express. The following
> > outlines the trouble (and successes) I've had so far.
>
> I assume your building from up-to-date git here?
I was using 0.9.1. I should switch to git.
> '@//.libvirt/libvirt-sock' should actually look like this
> '@/home//.libvirt/libvirt-sock' as you're running libvirtd
> as non-root it tries to open a UNIX socket in the home directory of
> the user starting it. This path is build via this pattern:
>
> @/.libvirt/libvirt-sock
I was actually running it as root.
Richard
>>>
>>> That's even stranger. libvirtd uses geteuid() == 0 to detect if it's
>>> running as root and acts upon that. It only tries to open a UNIX
>>> socket in the user's home (what it does in your case) when it detects
>>> non-root execution. Something is wrong here, but I've no clue what.
>>>
>>> Matthias
>>
>> Only linux supports the abstract socket namespace.
>> I ran into the same issue on OS X
>> (http://www.redhat.com/archives/libvir-list/2010-October/msg00969.html)
>>
>> Kind regards,
>>
>> Ruben
>
> Okay, that's a related but different problem, I think.
>
> As far as I understood the situation here Richard is running libvirtd
> as root. In that case libvirt should create the UNIX socket in
> [/usr/local]/var/run/libvirt/libvirt-sock. Only when running libvirtd
> as non-root it creates the UNIX socket in the abstract namespace, but
> not in the roor case. Therefore, running libvirtd as root should work
> on Solaris. But libvirtd seem to fail to detect being executed as
> root. It tries to create the UNIX socket in a broken path in the
> abstract namespace and this fails, but this is just a symptom, not the
> actual problem.
>
> The question is why, libvirtd thinks it's running as non-root while
> Richard says that he's running it as root.
>
> Matthias
It has probably something to do with this piece of code, in daemon/libvirtd.c:
#ifdef __sun
static int
qemudSetupPrivs (void)
{
chown ("/var/run/libvirt", SYSTEM_UID, SYSTEM_UID);
if (__init_daemon_priv (PU_RESETGROUPS | PU_CLEARLIMITSET,
SYSTEM_UID, SYSTEM_UID, PRIV_XVM_CONTROL, NULL)) {
VIR_ERROR(_("additional privileges are required"));
return -1;
}
if (priv_set (PRIV_OFF, PRIV_ALLSETS, PRIV_FILE_LINK_ANY, PRIV_PROC_INFO,
PRIV_PROC_SESSION, PRIV_PROC_EXEC, PRIV_PROC_FORK, NULL)) {
VIR_ERROR(_("failed to set reduced privileges"));
return -1;
}
return 0;
}
#else
# define qemudSetupPrivs() 0
#endif
This drops the privileges to those of the xvm user (SYSTEM_UID = 60)
After that, in qemudInitialize(), geteuid() returns 60 and
server->privileged is set to 0.
Since server->privileged is 0, we try to create the abstract socket,
which causes the error Richard is seeing.
This looks like a side-effect from commit a71f79c3
Makes sense?
Thanks,
Ruben
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 11/10] migrate: detect xml incompatibility
Detected by Coverity. Bug introduced in 08106e2044 (unreleased).
* src/conf/domain_conf.c (virDomainChannelDefCheckABIStability):
Use correct sizeof operand.
---
src/conf/domain_conf.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9975bca..0d9fef4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7269,7 +7269,8 @@ static bool
virDomainChannelDefCheckABIStability(virDomainChrDefPtr src,
}
break;
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD:
-if (memcmp(src->target.addr, dst->target.addr,
sizeof(src->target.addr)) != 0) {
+if (memcmp(src->target.addr, dst->target.addr,
+ sizeof(*src->target.addr)) != 0) {
char *saddr = virSocketFormatAddrFull(src->target.addr, true, ":");
char *daddr = virSocketFormatAddrFull(dst->target.addr, true, ":");
virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 10/10] build: silence coverity false positive
Coverity couldn't see that priv is clean on failure. But on failure, we might as well guarantee that callers don't try to free uninitialized memory. * src/remote/remote_driver.c (remoteGenericOpen): Even on failure, pass priv back to caller. --- In reading my own mail, I decided that patch 10 can be made simpler. v2: rewrite to avoid sa_assert and be more robust. src/remote/remote_driver.c |6 ++ 1 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 14c3d24..8335a1a 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2545,10 +2545,8 @@ remoteGenericOpen(virConnectPtr conn, virConnectAuthPtr auth, * use the UNIX transport. This handles Xen driver * which doesn't have its own impl of the network APIs. */ struct private_data *priv; -int ret; -ret = remoteOpenSecondaryDriver(conn, auth, flags, &priv); -if (ret == VIR_DRV_OPEN_SUCCESS) -*genericPrivateData = priv; +int ret = remoteOpenSecondaryDriver(conn, auth, flags, &priv); +*genericPrivateData = priv; return ret; } } -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 10/10] build: silence coverity false positive
* src/remote/remote_driver.c (remoteGenericOpen): Help coverity.
---
src/remote/remote_driver.c |7 ++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 14c3d24..fcb4c14 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -2547,8 +2547,13 @@ remoteGenericOpen(virConnectPtr conn, virConnectAuthPtr
auth,
struct private_data *priv;
int ret;
ret = remoteOpenSecondaryDriver(conn, auth, flags, &priv);
-if (ret == VIR_DRV_OPEN_SUCCESS)
+if (ret == VIR_DRV_OPEN_SUCCESS) {
*genericPrivateData = priv;
+} else {
+/* Coverity 5.3.0 missed that remoteOpenSecondaryDriver
+ * guarantees priv is clean on failure. */
+sa_assert(!priv);
+}
return ret;
}
}
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 09/10] build: silence coverity false positive
Coverity complained that infd could be -1 at the point where it is
passed to write, when in reality, this code can only be reached if
infd is non-negative.
* src/util/command.c (virCommandProcessIO): Help out coverity.
---
src/util/command.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/src/util/command.c b/src/util/command.c
index a2f7ff6..b51bdcf 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -961,6 +961,9 @@ virCommandProcessIO(virCommandPtr cmd)
} else {
int done;
+/* Coverity 5.3.0 can't see that we only get here if
+ * infd is in the set because it was non-negative. */
+sa_assert(infd != -1);
done = write(infd, cmd->inbuf + inoff,
inlen - inoff);
if (done < 0) {
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/10] event: avoid memory leak on cleanup
Detected by Coverity. Introduced in commit aaf2b70, and turned into a regression in the next few commits through 4e6e6672 (unreleased). * src/conf/domain_event.c (virDomainEventStateFree): Free object, per documentation. --- src/conf/domain_event.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 34a9d91..fabc1a5 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -1,7 +1,7 @@ /* * domain_event.c: domain event queue processing helpers * - * Copyright (C) 2010 Red Hat, Inc. + * Copyright (C) 2010-2011 Red Hat, Inc. * Copyright (C) 2008 VirtualIron * * This library is free software; you can redistribute it and/or @@ -555,6 +555,7 @@ virDomainEventStateFree(virDomainEventStatePtr state) if (state->timer != -1) virEventRemoveTimeout(state->timer); +VIR_FREE(state); } /** -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 07/10] qemu: avoid memory leak on vcpupin
Detected by Coverity. This leaked a cpumap on every iteration
of the loop. Leak introduced in commit 1cc4d02 (v0.9.0).
* src/qemu/qemu_process.c (qemuProcessSetVcpuAffinites): Plug
leak, and hoist allocation outside loop.
---
src/qemu/qemu_process.c | 21 +
1 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 116253e..f175d50 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1195,6 +1195,8 @@ qemuProcessSetVcpuAffinites(virConnectPtr conn,
pid_t vcpupid;
unsigned char *cpumask;
int vcpu, cpumaplen, hostcpus, maxcpu;
+unsigned char *cpumap = NULL;
+int ret = -1;
if (virNodeGetInfo(conn, &nodeinfo) != 0) {
return -1;
@@ -1216,18 +1218,18 @@ qemuProcessSetVcpuAffinites(virConnectPtr conn,
if (maxcpu > hostcpus)
maxcpu = hostcpus;
+if (VIR_ALLOC_N(cpumap, cpumaplen) < 0) {
+virReportOOMError();
+return -1;
+}
+
for (vcpu = 0; vcpu < def->cputune.nvcpupin; vcpu++) {
if (vcpu != def->cputune.vcpupin[vcpu]->vcpuid)
continue;
int i;
-unsigned char *cpumap = NULL;
-
-if (VIR_ALLOC_N(cpumap, cpumaplen) < 0) {
-virReportOOMError();
-return -1;
-}
+memset(cpumap, 0, cpumaplen);
cpumask = (unsigned char *)def->cputune.vcpupin[vcpu]->cpumask;
vcpupid = priv->vcpupids[vcpu];
@@ -1249,11 +1251,14 @@ qemuProcessSetVcpuAffinites(virConnectPtr conn,
cpumap,
cpumaplen,
maxcpu) < 0) {
-return -1;
+goto cleanup;
}
}
-return 0;
+ret = 0;
+cleanup:
+VIR_FREE(cpumap);
+return ret;
}
static int
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 06/10] remote: avoid leak on failure
Detected by Coverity. Only possible in OOM situations.
* daemon/remote.c (remoteDispatchDomainScreenshot): Plug leak.
---
daemon/remote.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 2a32ee8..49058f2 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1453,6 +1453,7 @@ remoteDispatchDomainScreenshot(struct qemud_server
*server ATTRIBUTE_UNUSED,
*mime_p = strdup(mime);
if (*mime_p == NULL) {
virReportOOMError();
+VIR_FREE(mime_p);
goto cleanup;
}
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 05/10] lock: avoid leak on failure
Detected by Coverity. Only possible on OOM situations.
* src/locking/lock_manager.c (virLockManagerPluginNew): Plug leak.
---
src/locking/lock_manager.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/locking/lock_manager.c b/src/locking/lock_manager.c
index 6197fd4..138cc91 100644
--- a/src/locking/lock_manager.c
+++ b/src/locking/lock_manager.c
@@ -120,7 +120,7 @@ virLockManagerPluginPtr virLockManagerPluginNew(const char
*name,
{
void *handle = NULL;
virLockDriverPtr driver;
-virLockManagerPluginPtr plugin;
+virLockManagerPluginPtr plugin = NULL;
const char *moddir = getenv("LIBVIRT_LOCK_MANAGER_PLUGIN_DIR");
char *modfile = NULL;
@@ -182,6 +182,7 @@ virLockManagerPluginPtr virLockManagerPluginNew(const char
*name,
return plugin;
cleanup:
+VIR_FREE(plugin);
VIR_FREE(modfile);
if (handle)
dlclose(handle);
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/10] storage: avoid memory leak on stat failure
Spotted by coverity. Triggers on failed stat, although I'm not sure
how easy that condition is, so I'm not sure if this is a runtime
memory hog. Regression introduced in commit 8077d64 (unreleased).
* src/util/storage_file.c (virStorageFileGetMetadataFromFD):
Reduce need for malloc, avoiding a leak.
---
src/util/storage_file.c | 15 +++
1 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/src/util/storage_file.c b/src/util/storage_file.c
index 8dbd933..6b3b756 100644
--- a/src/util/storage_file.c
+++ b/src/util/storage_file.c
@@ -831,11 +831,6 @@ virStorageFileGetMetadataFromFD(const char *path,
int ret = -1;
struct stat sb;
-if (VIR_ALLOC_N(head, len) < 0) {
-virReportOOMError();
-return -1;
-}
-
memset(meta, 0, sizeof (*meta));
if (fstat(fd, &sb) < 0) {
@@ -847,13 +842,17 @@ virStorageFileGetMetadataFromFD(const char *path,
/* No header to probe for directories */
if (S_ISDIR(sb.st_mode)) {
-ret = 0;
-goto cleanup;
+return 0;
}
if (lseek(fd, 0, SEEK_SET) == (off_t)-1) {
virReportSystemError(errno, _("cannot seek to start of '%s'"), path);
-goto cleanup;
+return -1;
+}
+
+if (VIR_ALLOC_N(head, len) < 0) {
+virReportOOMError();
+return -1;
}
if ((len = read(fd, head, len)) < 0) {
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 01/10] command: avoid leak on failure
Detected by Coverity. While it is possible on OOM condition, as well as with bad code that passes binary == NULL, it is unlikely to be encountered in the wild. * src/util/command.c (virCommandNewArgList): Don't leak memory. --- src/util/command.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/util/command.c b/src/util/command.c index 288958e..a2f7ff6 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -142,7 +142,7 @@ virCommandNewArgList(const char *binary, ...) const char *arg; if (!cmd || cmd->has_error) -return NULL; +return cmd; va_start(list, binary); while ((arg = va_arg(list, const char *)) != NULL) -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 00/10] coverity cleanups, round 1
Coverity reported a number of bugs against libvirt. I ran out of time to get through them all today, but there are certainly some real doozies in here. I've blamed commits where non-trivial bugs were introduced, to help distros decide if things need back-porting (I think that leaks on OOM situations and coverity false positives are not serious enough to warrant a back-port). These patches are pretty much independent (can be applied in any order); I suppose I could have rebased it to order the series by severity, oh well. Eric Blake (10): command: avoid leak on failure libvirtd: avoid leak on failure storage: avoid memory leak storage: avoid memory leak on stat failure lock: avoid leak on failure remote: avoid leak on failure qemu: avoid memory leak on vcpupin event: avoid memory leak on cleanup build: silence coverity false positive build: silence coverity false positive daemon/libvirtd.c|4 +++- daemon/remote.c |1 + src/conf/domain_event.c |3 ++- src/locking/lock_manager.c |3 ++- src/qemu/qemu_process.c | 21 + src/remote/remote_driver.c |7 ++- src/storage/storage_backend_fs.c | 15 +-- src/util/command.c |5 - src/util/storage_file.c | 15 +++ 9 files changed, 39 insertions(+), 35 deletions(-) -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 02/10] libvirtd: avoid leak on failure
Spotted by Coverity. Only possible on an OOM condition, so
unlikely to bite in the wild.
* daemon/libvirtd.c (qemudSetLogging): Don't leak memory.
---
daemon/libvirtd.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index aec81cf..728031f 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -2779,8 +2779,10 @@ qemudSetLogging(struct qemud_server *server, virConfPtr
conf,
goto free_and_fail;
if (virAsprintf(&tmp, "%d:file:%s/.libvirt/libvirtd.log",
-virLogGetDefaultPriority(), userdir) == -1)
+virLogGetDefaultPriority(), userdir) == -1) {
+VIR_FREE(userdir);
goto out_of_memory;
+}
}
} else {
if (virAsprintf(&tmp, "%d:stderr", virLogGetDefaultPriority()) < 0)
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/10] storage: avoid memory leak
Coverity detected that options was being set by strdup but never
freed. But why even bother with an options variable? The options
parameter never changes! Leak present since commit 44948f5b (0.7.0).
This function could probably be rewritten to take better advantage
of virCommand, but that is more invasive.
* src/storage/storage_backend_fs.c
(virStorageBackendFileSystemMount): Avoid wasted strdup, and
guarantee proper cleanup on all paths.
---
src/storage/storage_backend_fs.c | 15 +--
1 files changed, 1 insertions(+), 14 deletions(-)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 3f4d978..207669a 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -317,7 +317,6 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr
pool) {
static int
virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) {
char *src;
-char *options = NULL;
const char **mntargv;
/* 'mount -t auto' doesn't seem to auto determine nfs (or cifs),
@@ -328,7 +327,6 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool)
{
int glusterfs = (pool->def->type == VIR_STORAGE_POOL_NETFS &&
pool->def->source.format == VIR_STORAGE_POOL_NETFS_GLUSTERFS);
-int option_index;
int source_index;
const char *netfs_auto_argv[] = {
@@ -358,7 +356,7 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool)
{
virStoragePoolFormatFileSystemNetTypeToString(pool->def->source.format),
NULL,
"-o",
-NULL,
+"direct-io-mode=1",
pool->def->target.path,
NULL,
};
@@ -369,7 +367,6 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool)
{
} else if (glusterfs) {
mntargv = glusterfs_argv;
source_index = 3;
-option_index = 5;
} else {
mntargv = fs_argv;
source_index = 3;
@@ -405,12 +402,6 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr
pool) {
}
if (pool->def->type == VIR_STORAGE_POOL_NETFS) {
-if (pool->def->source.format == VIR_STORAGE_POOL_NETFS_GLUSTERFS) {
-if ((options = strdup("direct-io-mode=1")) == NULL) {
-virReportOOMError();
-return -1;
-}
-}
if (virAsprintf(&src, "%s:%s",
pool->def->source.host.name,
pool->def->source.dir) == -1) {
@@ -426,10 +417,6 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr
pool) {
}
mntargv[source_index] = src;
-if (glusterfs) {
-mntargv[option_index] = options;
-}
-
if (virRun(mntargv, NULL) < 0) {
VIR_FREE(src);
return -1;
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix minor issues in libxenlight managed save
Eric Blake wrote: > On 06/02/2011 04:04 PM, Jim Fehlig wrote: > >> There were a few minor issues in commit 5b6c961e >> - leak managed save path >> - leak managed save fd >> - functions that open an fd should also close it >> --- >> src/libxl/libxl_driver.c | 30 -- >> 1 files changed, 20 insertions(+), 10 deletions(-) >> > > ACK. > Thanks, pushed. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix minor issues in libxenlight managed save
On 06/02/2011 04:04 PM, Jim Fehlig wrote: > There were a few minor issues in commit 5b6c961e > - leak managed save path > - leak managed save fd > - functions that open an fd should also close it > --- > src/libxl/libxl_driver.c | 30 -- > 1 files changed, 20 insertions(+), 10 deletions(-) ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix minor issues in libxenlight managed save
There were a few minor issues in commit 5b6c961e
- leak managed save path
- leak managed save fd
- functions that open an fd should also close it
---
src/libxl/libxl_driver.c | 30 --
1 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 9a794e8..ed24d10 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -632,23 +632,27 @@ libxlVmStart(libxlDriverPrivatePtr driver,
virDomainObjPtr vm,
int ret;
uint32_t domid = 0;
char *dom_xml = NULL;
-char *managed_save = NULL;
+char *managed_save_path = NULL;
+int managed_save_fd = -1;
pid_t child_console_pid = -1;
libxlDomainObjPrivatePtr priv = vm->privateData;
/* If there is a managed saved state restore it instead of starting
* from scratch. The old state is removed once the restoring succeeded. */
if (restore_fd < 0) {
-managed_save = libxlDomainManagedSavePath(driver, vm);
-if (managed_save == NULL)
+managed_save_path = libxlDomainManagedSavePath(driver, vm);
+if (managed_save_path == NULL)
goto error;
-if (virFileExists(managed_save)) {
+if (virFileExists(managed_save_path)) {
-restore_fd = libxlSaveImageOpen(driver, managed_save, &def, &hdr);
-if (restore_fd < 0)
+managed_save_fd = libxlSaveImageOpen(driver, managed_save_path,
+ &def, &hdr);
+if (managed_save_fd < 0)
goto error;
+restore_fd = managed_save_fd;
+
if (STRNEQ(vm->def->name, def->name) ||
memcmp(vm->def->uuid, def->uuid, VIR_UUID_BUFLEN)) {
char vm_uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -665,9 +669,12 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr
vm,
virDomainObjAssignDef(vm, def, true);
def = NULL;
-if (unlink(managed_save) < 0)
-VIR_WARN("Failed to remove the managed state %s",
managed_save);
+if (unlink(managed_save_path) < 0) {
+VIR_WARN("Failed to remove the managed state %s",
+ managed_save_path);
+}
}
+VIR_FREE(managed_save_path);
}
memset(&d_config, 0, sizeof(d_config));
@@ -738,6 +745,7 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr
vm,
libxl_domain_config_destroy(&d_config);
VIR_FREE(dom_xml);
+VIR_FORCE_CLOSE(managed_save_fd);
return 0;
error:
@@ -748,9 +756,9 @@ error:
}
libxl_domain_config_destroy(&d_config);
VIR_FREE(dom_xml);
-VIR_FREE(managed_save);
+VIR_FREE(managed_save_path);
virDomainDefFree(def);
-VIR_FORCE_CLOSE(restore_fd);
+VIR_FORCE_CLOSE(managed_save_fd);
return -1;
}
@@ -1955,6 +1963,8 @@ libxlDomainRestore(virConnectPtr conn, const char *from)
}
cleanup:
+if (VIR_CLOSE(fd) < 0)
+virReportSystemError(errno, "%s", _("cannot close file"));
virDomainDefFree(def);
if (vm)
virDomainObjUnlock(vm);
--
1.7.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] Add managedSave support to libxl driver
Markus Groß wrote:
> ---
> src/libxl/libxl_driver.c | 353
> --
> 1 files changed, 276 insertions(+), 77 deletions(-)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 61c3494..9bcc3b9 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -66,7 +66,6 @@ static int
> libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
> bool start_paused, int restore_fd);
>
> -
> /* Function definitions */
> static void
> libxlDriverLock(libxlDriverPrivatePtr driver)
> @@ -219,6 +218,87 @@ libxlDoNodeGetInfo(libxlDriverPrivatePtr driver,
> virNodeInfoPtr info)
> return 0;
> }
>
> +static char *
> +libxlDomainManagedSavePath(libxlDriverPrivatePtr driver, virDomainObjPtr vm)
> {
> +char *ret;
> +
> +if (virAsprintf(&ret, "%s/%s.save", driver->saveDir, vm->def->name) < 0)
> {
> +virReportOOMError();
> +return NULL;
> +}
> +
> +return ret;
> +}
> +
> +/* This internal function expects the driver lock to already be held on
> + * entry. */
> +static int ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4)
> +libxlSaveImageOpen(libxlDriverPrivatePtr driver, const char *from,
> + virDomainDefPtr *ret_def, libxlSavefileHeaderPtr
> ret_hdr)
> +{
> +int fd;
> +virDomainDefPtr def = NULL;
> +libxlSavefileHeader hdr;
> +char *xml = NULL;
> +
> +if ((fd = virFileOpenAs(from, O_RDONLY, 0, getuid(), getgid(), 0)) < 0) {
> +libxlError(VIR_ERR_OPERATION_FAILED,
> + "%s", _("cannot read domain image"));
> +goto error;
> +}
> +
> +if (saferead(fd, &hdr, sizeof(hdr)) != sizeof(hdr)) {
> +libxlError(VIR_ERR_OPERATION_FAILED,
> + "%s", _("failed to read libxl header"));
> +goto error;
> +}
> +
> +if (memcmp(hdr.magic, LIBXL_SAVE_MAGIC, sizeof(hdr.magic))) {
> +libxlError(VIR_ERR_INVALID_ARG, "%s", _("image magic is incorrect"));
> +goto error;
> +}
> +
> +if (hdr.version > LIBXL_SAVE_VERSION) {
> +libxlError(VIR_ERR_OPERATION_FAILED,
> + _("image version is not supported (%d > %d)"),
> + hdr.version, LIBXL_SAVE_VERSION);
> +goto error;
> +}
> +
> +if (hdr.xmlLen <= 0) {
> +libxlError(VIR_ERR_OPERATION_FAILED,
> + _("invalid XML length: %d"), hdr.xmlLen);
> +goto error;
> +}
> +
> +if (VIR_ALLOC_N(xml, hdr.xmlLen) < 0) {
> +virReportOOMError();
> +goto error;
> +}
> +
> +if (saferead(fd, xml, hdr.xmlLen) != hdr.xmlLen) {
> +libxlError(VIR_ERR_OPERATION_FAILED, "%s", _("failed to read XML"));
> +goto error;
> +}
> +
> +if (!(def = virDomainDefParseString(driver->caps, xml,
> +VIR_DOMAIN_XML_INACTIVE)))
> +goto error;
> +
> +VIR_FREE(xml);
> +
> +*ret_def = def;
> +*ret_hdr = hdr;
> +
> +return fd;
> +
> +error:
> +VIR_FREE(xml);
> +virDomainDefFree(def);
> +VIR_FORCE_CLOSE(fd);
> +return -1;
> +}
> +
> /*
> * Cleanup function for domain that has reached shutoff state.
> *
> @@ -546,17 +626,53 @@ libxlVmStart(libxlDriverPrivatePtr driver,
> virDomainObjPtr vm,
> bool start_paused, int restore_fd)
> {
> libxl_domain_config d_config;
> -virDomainDefPtr def = vm->def;
> +virDomainDefPtr def = NULL;
> virDomainEventPtr event = NULL;
> +libxlSavefileHeader hdr;
> int ret;
> uint32_t domid = 0;
> char *dom_xml = NULL;
> +char *managed_save = NULL;
> pid_t child_console_pid = -1;
> libxlDomainObjPrivatePtr priv = vm->privateData;
>
> +/* If there is a managed saved state restore it instead of starting
> + * from scratch. The old state is removed once the restoring succeeded.
> */
> +if (restore_fd < 0) {
> +managed_save = libxlDomainManagedSavePath(driver, vm);
> +if (managed_save == NULL)
> +goto error;
> +
> +if (virFileExists(managed_save)) {
> +
> +restore_fd = libxlSaveImageOpen(driver, managed_save, &def,
> &hdr);
> +if (restore_fd < 0)
> +goto error;
> +
> +if (STRNEQ(vm->def->name, def->name) ||
> +memcmp(vm->def->uuid, def->uuid, VIR_UUID_BUFLEN)) {
> +char vm_uuidstr[VIR_UUID_STRING_BUFLEN];
> +char def_uuidstr[VIR_UUID_STRING_BUFLEN];
> +virUUIDFormat(vm->def->uuid, vm_uuidstr);
> +virUUIDFormat(def->uuid, def_uuidstr);
> +libxlError(VIR_ERR_OPERATION_FAILED,
> + _("cannot restore domain '%s' uuid %s from a file"
> + " which belongs to domain '%s' uuid %s"),
> + vm->def->name, vm_uuidstr, def->name,
> def_uuidstr);
> +goto error;
> +}
> +
> +
Re: [libvirt] [Qemu-devel] QMP: RFC: I/O error info & query-stop-reason
On 06/02/2011 03:13 PM, Eric Blake wrote:
On 06/02/2011 02:03 PM, Anthony Liguori wrote:
{ "event": "BLOCK_IO_ERROR",
"data": { "device": "ide0-hd1",
"operation": "write",
"action": "stop",
"reason": "enospc", }
I'm ok with either way. But in case you meant the second one, I guess
we should make "reason" a dictionary so that we can group related
information when we extend the field, for example:
"reason": { "no space": false, "no permission": true }
The idea for an event with details certainly has merit.
In an ideal would, would would just embed the BlockDeviceInfo structure
in the event and call it a day. In a less than ideal world, I think
it's better to make a call to query-block after having received the
BLOCK_IO_ERROR event.
Why would we ever have "no permission"?
SELinux denial, perhaps?
Maybe, but that would take a considerable amount of magic to make happen
in practice. Really only sounds plausible as an sVirt bug. I think you
could only make this happen if you you doing dynamic labelling.
Regards,
Anthony Liguori
Maybe libvirt guys could provide more input wrt the error reason usage.
If we don't have valid use cases for other errors, then I'll agree that
providing only "no space" is enough.
Definitely! Adding libvirt to the CC to help encourage their input.
We could always start with just one reason "no space", and add more
later if and when we come up for use cases.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/7] lxc: Ensure container actually exists
On 06/02/2011 01:40 PM, Cole Robinson wrote: > Since we can't really get useful error reporting from virCommandExec since > it needs to be the last thing we do. > > Signed-off-by: Cole Robinson > --- > src/lxc/lxc_container.c |7 +++ > 1 files changed, 7 insertions(+), 0 deletions(-) ACK; again up to you if you want to count this as a bug fix and push for 0.9.2. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/7] lxc: Verify root fs exists before mounting
On 06/02/2011 01:40 PM, Cole Robinson wrote:
> Otherwise the following virFileMakePath will create the directory for
> us and fail further ahead, which probably isn't intended.
>
> Signed-off-by: Cole Robinson
> ---
> src/lxc/lxc_controller.c |8
> 1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
> index c94d0d0..7d60090 100644
> --- a/src/lxc/lxc_controller.c
> +++ b/src/lxc/lxc_controller.c
> @@ -664,6 +664,14 @@ lxcControllerRun(virDomainDefPtr def,
> */
> if (root) {
> VIR_DEBUG("Setting up private /dev/pts");
> +
> +if (!virFileExists(root->src)) {
> +virReportSystemError(errno,
> + _("root source %s does not exist"),
> + root->src);
> +goto cleanup;
> +}
ACK. This one's simple enough and qualifies as a bug-fix that you could
rebase it to push now prior to 0.9.2; or you can just wait until the
release and push with the rest of the series.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/7] lxc: controller: Improve container error reporting
On 06/02/2011 01:40 PM, Cole Robinson wrote:
> Add a handshake with the cloned container process to try and detect
> if it fails to start.
>
> Signed-off-by: Cole Robinson
> ---
> src/lxc/lxc_container.c | 18 ++
> src/lxc/lxc_container.h |1 +
> src/lxc/lxc_controller.c | 17 +
> 3 files changed, 32 insertions(+), 4 deletions(-)
Interesting - pipe() in the last patch, and socketpair() in this patch.
At any rate, the underlying use of the fd pair is the same.
ACK, post-release.
> @@ -630,6 +631,12 @@ lxcControllerRun(virDomainDefPtr def,
> goto cleanup;
> }
>
> +if (socketpair(PF_UNIX, SOCK_STREAM, 0, containerhandshake) < 0) {
> +virReportSystemError(errno, "%s",
> + _("socketpair failed"));
> +goto cleanup;
> +}
> +
> root = virDomainGetRootFilesystem(def);
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/7] lxc: Improve guest startup error reporting
On 06/02/2011 01:40 PM, Cole Robinson wrote:
> Add a simple handshake with the lxc_controller process so we can detect
> process startup failures. We do this by adding a new --handshake cli arg
> to lxc_controller for passing a file descriptor. If the process fails to
> launch, we scrape all output from the logfile and report it to the user.
>
> Signed-off-by: Cole Robinson
> ---
> src/lxc/lxc_container.c |2 +-
> src/lxc/lxc_container.h |1 +
> src/lxc/lxc_controller.c | 33 +-
> src/lxc/lxc_driver.c | 107
> +-
> 4 files changed, 137 insertions(+), 6 deletions(-)
>
> @@ -1489,6 +1572,12 @@ static int lxcVmStart(virConnectPtr conn,
> if (virCommandRun(cmd, NULL) < 0)
> goto cleanup;
>
> +if (VIR_CLOSE(handshakefds[1]) < 0) {
> +virReportSystemError(errno, "%s", _("could not close handshake fd"));
> +goto cleanup;
> +}
> +handshakefds[1] = -1;
This line is redundant; the VIR_CLOSE() above already set this to -1.
ACK with that nit fixed, post-release.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/7] lxc: Refactor controller command building
On 06/02/2011 01:40 PM, Cole Robinson wrote: > Arranges things similar to the qemu driver. Will allow us to more easilly s/easilly/easily/ > report command error output. > > Signed-off-by: Cole Robinson > --- > src/lxc/lxc_driver.c | 74 ++--- > 1 files changed, 39 insertions(+), 35 deletions(-) > ACK post-release; looks like simple code motion. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] QMP: RFC: I/O error info & query-stop-reason
On 06/02/2011 02:03 PM, Anthony Liguori wrote:
{ "event": "BLOCK_IO_ERROR",
"data": { "device": "ide0-hd1",
"operation": "write",
"action": "stop",
"reason": "enospc", }
>>>
>> I'm ok with either way. But in case you meant the second one, I guess
>> we should make "reason" a dictionary so that we can group related
>> information when we extend the field, for example:
>>
>> "reason": { "no space": false, "no permission": true }
The idea for an event with details certainly has merit.
>
> Why would we ever have "no permission"?
SELinux denial, perhaps?
>
>> Maybe libvirt guys could provide more input wrt the error reason usage.
>> If we don't have valid use cases for other errors, then I'll agree that
>> providing only "no space" is enough.
>
> Definitely! Adding libvirt to the CC to help encourage their input.
We could always start with just one reason "no space", and add more
later if and when we come up for use cases.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/7] lxc: Don't report error in Wait/SendContinue
On 06/02/2011 01:40 PM, Cole Robinson wrote: > We will reuse these shortly, and each use should have a different error > message. > > Signed-off-by: Cole Robinson > --- > src/lxc/lxc_container.c | 15 ++- > src/lxc/lxc_controller.c |5 - > 2 files changed, 10 insertions(+), 10 deletions(-) So basically you are hoisting the error message out of the low-level routine into the caller, for preparation of reusing the low-level routine from different contexts where the caller can print a better message. ACK, but again post-release. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] QMP: RFC: I/O error info & query-stop-reason
On 06/02/2011 02:13 PM, Luiz Capitulino wrote:
On Thu, 02 Jun 2011 13:33:52 -0500
Anthony Liguori wrote:
On 06/02/2011 01:09 PM, Luiz Capitulino wrote:
On Thu, 02 Jun 2011 13:00:04 -0500
Anthony Liguori wrote:
On 06/02/2011 12:57 PM, Luiz Capitulino wrote:
On Wed, 01 Jun 2011 16:35:03 -0500
Anthony Liguoriwrote:
On 06/01/2011 04:12 PM, Luiz Capitulino wrote:
Hi there,
There are people who want to use QMP for thin provisioning. That's, the VM is
started with a small storage and when a no space error is triggered, more space
is allocated and the VM is put to run again.
QMP has two limitations that prevent people from doing this today:
1. The BLOCK_IO_ERROR doesn't contain error information
2. Considering we solve item 1, we still have to provide a way for clients
to query why a VM stopped. This is needed because clients may miss the
BLOCK_IO_ERROR event or may connect to the VM while it's already stopped
A proposal to solve both problems follow.
A. BLOCK_IO_ERROR information
-
We already have discussed this a lot, but didn't reach a consensus. My solution
is quite simple: to add a stringfied errno name to the BLOCK_IO_ERROR event,
for example (see the "reason" key):
{ "event": "BLOCK_IO_ERROR",
"data": { "device": "ide0-hd1",
"operation": "write",
"action": "stop",
"reason": "enospc", }
you can call the reason whatever you want, but don't call it stringfied
errno name :-)
In fact, just make reason "no space".
You mean, we should do:
"reason": "no space"
Or that we should make it a boolean, like:
"no space": true
Do we need reason in BLOCK_IO_ERROR if query-block returns this information?
True, no.
I'm ok with either way. But in case you meant the second one, I guess
we should make "reason" a dictionary so that we can group related
information when we extend the field, for example:
"reason": { "no space": false, "no permission": true }
Why would we ever have "no permission"?
Why did it happen? It's not clear to me when read/write would return
EPERM. open() should fail. In fact, EPERM is not mentioned in man 2 read.
Actually, the error was an EACCESS which might sound more bizarre :)
What happened was that the device file in question had its permission
changed during VM execution due to a bug somewhere else. I'm not sure if
the error was returned in a read() or write() (Kevin might have more details).
Strange, EACCES should only happen on open(). Is it possible that
somehow a reopen was happening?
This is a bit extreme and I'd agree it's arguable whether or not we should
report EACCESS, but I had this in mind and ended up mentioning it...
If we can't explain why an error would occur, we shouldn't make it part
of the protocol :-)
Maybe libvirt guys could provide more input wrt the error reason usage.
If we don't have valid use cases for other errors, then I'll agree that
providing only "no space" is enough.
Definitely! Adding libvirt to the CC to help encourage their input.
Regards,
Anthony Liguori
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/7] lxc: Drop container stdio as late as possible
On 06/02/2011 01:40 PM, Cole Robinson wrote:
> Makes it more likely we get useful error output in the logs
>
> Signed-off-by: Cole Robinson
> ---
> src/lxc/lxc_container.c | 29 -
> 1 files changed, 16 insertions(+), 13 deletions(-)
ACK; however, this series is late enough that it should wait until after
the 0.9.2 release.
> @@ -806,17 +798,28 @@ static int lxcContainerChild( void *data )
>
> /* rename and enable interfaces */
> if (lxcContainerRenameAndEnableInterfaces(argv->nveths,
> - argv->veths) < 0)
> + argv->veths) < 0) {
> goto cleanup;
> +}
This particular change doesn't hurt or help; but I'm okay with leaving
it in from the consistency standpoint of using {} everywhere.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] docs: document iface-* commands
On 06/02/2011 09:56 AM, Laine Stump wrote: > On 05/31/2011 04:43 PM, Eric Blake wrote: >> I intentionally set things up so 'virsh help interface' lists >> commands in alphabetical order, but 'man virsh' lists them in >> topical order; this matches our practice on some other commands. >> >> * tools/virsh.pod: Document all iface commands. >> * tools/virsh.c (ifaceCmds): Sort. >> + >> +A lot of the commands for host interfaces are similar to the ones used > > Same thing - s/A lot/Many/ > > ACK with those two small nits fixed. Thanks; adjusted and pushed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] to correct UML command line networking parameters
On 06/02/2011 10:23 AM, Heath Petersen wrote:
> I have been finding that some UML command line networking parameters are
> being generated incorrectly. Attached is a patch that corrects the
> problem.
>
> For more information, see
> https://bugzilla.redhat.com/show_bug.cgi?id=706295 .
Thanks for the patch. I ended up rewriting your patch, but I'm still
pushing this in your name:
>
> diff -Naur libvirt-0.9.1.orig/src/uml/uml_conf.c
> libvirt-0.9.1.new/src/uml/uml_conf.c
> --- libvirt-0.9.1.orig/src/uml/uml_conf.c 2011-03-01 01:03:32.0
> -0600
> +++ libvirt-0.9.1.new/src/uml/uml_conf.c 2011-05-27 20:27:26.563772975
> -0500
> @@ -208,6 +208,11 @@
> case VIR_DOMAIN_NET_TYPE_ETHERNET:
> /* ethNNN=tuntap,tapname,macaddr,gateway */
> virBufferAddLit(&buf, "tuntap");
> +if (def->ifname) {
> +virBufferVSprintf(&buf, ",%s", def->ifname);
Your patch was based off of 0.9.1, but it is better to base off of
libvirt.git HEAD; there, we renamed virBufferVSprintf -> virBufferAsprintf.
> +} else {
> +virBufferAddLit(&buf, ",");
virBufferAddChar is more efficient here.
But after I made those two tweaks, I noticed that it is even more
efficient to always print the comma as part of the previous literal:
diff --git i/src/uml/uml_conf.c w/src/uml/uml_conf.c
index be902a6..0122472 100644
--- i/src/uml/uml_conf.c
+++ w/src/uml/uml_conf.c
@@ -207,11 +207,9 @@ umlBuildCommandLineNet(virConnectPtr conn,
case VIR_DOMAIN_NET_TYPE_ETHERNET:
/* ethNNN=tuntap,tapname,macaddr,gateway */
-virBufferAddLit(&buf, "tuntap");
+virBufferAddLit(&buf, "tuntap,");
if (def->ifname) {
-virBufferAsprintf(&buf, ",%s", def->ifname);
-} else {
-virBufferAddChar(&buf, ',');
+virBufferAdd(&buf, def->ifname, -1);
}
if (def->data.ethernet.ipaddr) {
umlReportError(VIR_ERR_INTERNAL_ERROR, "%s",
I also added you to AUTHORS; feel free to let me know if you prefer any
alternate spelling.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/7] lxc: Drop container stdio as late as possible
Makes it more likely we get useful error output in the logs
Signed-off-by: Cole Robinson
---
src/lxc/lxc_container.c | 29 -
1 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 9ae93b5..173af07 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -751,9 +751,9 @@ static int lxcContainerChild( void *data )
{
lxc_child_argv_t *argv = data;
virDomainDefPtr vmDef = argv->config;
-int ttyfd;
+int ttyfd = -1;
int ret = -1;
-char *ttyPath;
+char *ttyPath = NULL;
virDomainFSDefPtr root;
virCommandPtr cmd = NULL;
@@ -786,16 +786,8 @@ static int lxcContainerChild( void *data )
virReportSystemError(errno,
_("Failed to open tty %s"),
ttyPath);
-VIR_FREE(ttyPath);
goto cleanup;
}
-VIR_FREE(ttyPath);
-
-if (lxcContainerSetStdio(argv->monitor, ttyfd) < 0) {
-VIR_FORCE_CLOSE(ttyfd);
-goto cleanup;
-}
-VIR_FORCE_CLOSE(ttyfd);
if (lxcContainerSetupMounts(vmDef, root) < 0)
goto cleanup;
@@ -806,17 +798,28 @@ static int lxcContainerChild( void *data )
/* rename and enable interfaces */
if (lxcContainerRenameAndEnableInterfaces(argv->nveths,
- argv->veths) < 0)
+ argv->veths) < 0) {
goto cleanup;
+}
/* drop a set of root capabilities */
if (lxcContainerDropCapabilities() < 0)
goto cleanup;
-/* this function will only return if an error occured */
-ret = virCommandExec(cmd);
+if (lxcContainerSetStdio(argv->monitor, ttyfd) < 0) {
+goto cleanup;
+}
+ret = 0;
cleanup:
+VIR_FREE(ttyPath);
+VIR_FORCE_CLOSE(ttyfd);
+
+if (ret == 0) {
+/* this function will only return if an error occured */
+ret = virCommandExec(cmd);
+}
+
virCommandFree(cmd);
return ret;
}
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/7] lxc: Verify root fs exists before mounting
Otherwise the following virFileMakePath will create the directory for
us and fail further ahead, which probably isn't intended.
Signed-off-by: Cole Robinson
---
src/lxc/lxc_controller.c |8
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index c94d0d0..7d60090 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -664,6 +664,14 @@ lxcControllerRun(virDomainDefPtr def,
*/
if (root) {
VIR_DEBUG("Setting up private /dev/pts");
+
+if (!virFileExists(root->src)) {
+virReportSystemError(errno,
+ _("root source %s does not exist"),
+ root->src);
+goto cleanup;
+}
+
if (unshare(CLONE_NEWNS) < 0) {
virReportSystemError(errno, "%s",
_("Cannot unshare mount namespace"));
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/7] lxc: Improve guest startup error reporting
Add a simple handshake with the lxc_controller process so we can detect
process startup failures. We do this by adding a new --handshake cli arg
to lxc_controller for passing a file descriptor. If the process fails to
launch, we scrape all output from the logfile and report it to the user.
Signed-off-by: Cole Robinson
---
src/lxc/lxc_container.c |2 +-
src/lxc/lxc_container.h |1 +
src/lxc/lxc_controller.c | 33 +-
src/lxc/lxc_driver.c | 107 +-
4 files changed, 137 insertions(+), 6 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 00add94..ff90842 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -213,7 +213,7 @@ error_out:
*
* Returns 0 on success or -1 in case of error
*/
-static int lxcContainerWaitForContinue(int control)
+int lxcContainerWaitForContinue(int control)
{
lxc_message_t msg;
int readLen;
diff --git a/src/lxc/lxc_container.h b/src/lxc/lxc_container.h
index 5e08d45..a3e457e 100644
--- a/src/lxc/lxc_container.h
+++ b/src/lxc/lxc_container.h
@@ -46,6 +46,7 @@ enum {
# define LXC_DEV_MAJ_PTY 136
int lxcContainerSendContinue(int control);
+int lxcContainerWaitForContinue(int control);
int lxcContainerStart(virDomainDefPtr def,
unsigned int nveths,
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index cef4b58..5bf8ee3 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -612,7 +612,8 @@ lxcControllerRun(virDomainDefPtr def,
char **veths,
int monitor,
int client,
- int appPty)
+ int appPty,
+ int handshakefd)
{
int rc = -1;
int control[2] = { -1, -1};
@@ -742,6 +743,13 @@ lxcControllerRun(virDomainDefPtr def,
if (lxcControllerClearCapabilities() < 0)
goto cleanup;
+if (lxcContainerSendContinue(handshakefd) < 0) {
+virReportSystemError(errno, "%s",
+ _("error sending continue signal to parent"));
+goto cleanup;
+}
+VIR_FORCE_CLOSE(handshakefd);
+
rc = lxcControllerMain(monitor, client, appPty, containerPty, container);
cleanup:
@@ -751,6 +759,7 @@ cleanup:
VIR_FORCE_CLOSE(control[1]);
VIR_FREE(containerPtyPath);
VIR_FORCE_CLOSE(containerPty);
+VIR_FORCE_CLOSE(handshakefd);
if (container > 1) {
int status;
@@ -774,6 +783,7 @@ int main(int argc, char *argv[])
char **veths = NULL;
int monitor = -1;
int appPty = -1;
+int handshakefd = -1;
int bg = 0;
virCapsPtr caps = NULL;
virDomainDefPtr def = NULL;
@@ -784,6 +794,7 @@ int main(int argc, char *argv[])
{ "name", 1, NULL, 'n' },
{ "veth", 1, NULL, 'v' },
{ "console", 1, NULL, 'c' },
+{ "handshakefd", 1, NULL, 's' },
{ "help", 0, NULL, 'h' },
{ 0, 0, 0, 0 },
};
@@ -798,7 +809,7 @@ int main(int argc, char *argv[])
while (1) {
int c;
-c = getopt_long(argc, argv, "dn:v:m:c:h",
+c = getopt_long(argc, argv, "dn:v:m:c:s:h",
options, NULL);
if (c == -1)
@@ -834,6 +845,14 @@ int main(int argc, char *argv[])
}
break;
+case 's':
+if (virStrToLong_i(optarg, NULL, 10, &handshakefd) < 0) {
+fprintf(stderr, "malformed --handshakefd argument '%s'",
+optarg);
+goto cleanup;
+}
+break;
+
case 'h':
case '?':
fprintf(stderr, "\n");
@@ -845,6 +864,7 @@ int main(int argc, char *argv[])
fprintf(stderr, " -n NAME, --name NAME\n");
fprintf(stderr, " -c FD, --console FD\n");
fprintf(stderr, " -v VETH, --veth VETH\n");
+fprintf(stderr, " -s FD, --handshakefd FD\n");
fprintf(stderr, " -h, --help\n");
fprintf(stderr, "\n");
goto cleanup;
@@ -862,6 +882,12 @@ int main(int argc, char *argv[])
goto cleanup;
}
+if (handshakefd < 0) {
+fprintf(stderr, "%s: missing --handshake argument for container PTY\n",
+argv[0]);
+goto cleanup;
+}
+
if (getuid() != 0) {
fprintf(stderr, "%s: must be run as the 'root' user\n", argv[0]);
goto cleanup;
@@ -932,7 +958,8 @@ int main(int argc, char *argv[])
goto cleanup;
}
-rc = lxcControllerRun(def, nveths, veths, monitor, client, appPty);
+rc = lxcControllerRun(def, nveths, veths, monitor, client, appPty,
+ handshakefd);
cleanup:
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 99f94ff..755de70 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1287,7 +1287,8 @@ lxcBuildControllerCmd(lxc_driver_t *driver,
int nveths,
[libvirt] [PATCH 7/7] lxc: Ensure container actually exists
Since we can't really get useful error reporting from virCommandExec since
it needs to be the last thing we do.
Signed-off-by: Cole Robinson
---
src/lxc/lxc_container.c |7 +++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 26b493e..7924858 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -785,6 +785,13 @@ static int lxcContainerChild( void *data )
if (lxcContainerSetupMounts(vmDef, root) < 0)
goto cleanup;
+if (!virFileExists(vmDef->os.init)) {
+virReportSystemError(errno,
+_("cannot find init path '%s' relative to container root"),
+vmDef->os.init);
+goto cleanup;
+}
+
/* Wait for interface devices to show up */
if (lxcContainerWaitForContinue(argv->monitor) < 0) {
virReportSystemError(errno, "%s",
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/7] lxc: Refactor controller command building
Arranges things similar to the qemu driver. Will allow us to more easilly
report command error output.
Signed-off-by: Cole Robinson
---
src/lxc/lxc_driver.c | 74 ++---
1 files changed, 39 insertions(+), 35 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 8eb87a2..99f94ff 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1281,21 +1281,18 @@ cleanup:
}
-static int lxcControllerStart(lxc_driver_t *driver,
- virDomainObjPtr vm,
- int nveths,
- char **veths,
- int appPty,
- int logfile)
+static virCommandPtr
+lxcBuildControllerCmd(lxc_driver_t *driver,
+ virDomainObjPtr vm,
+ int nveths,
+ char **veths,
+ int appPty,
+ int logfile)
{
int i;
-int ret = -1;
char *filterstr;
char *outputstr;
virCommandPtr cmd;
-off_t pos = -1;
-char ebuf[1024];
-char *timestamp;
cmd = virCommandNew(vm->def->emulator);
@@ -1357,33 +1354,14 @@ static int lxcControllerStart(lxc_driver_t *driver,
goto cleanup;
}
-/* Log timestamp */
-if ((timestamp = virTimestamp()) == NULL) {
-virReportOOMError();
-goto cleanup;
-}
-if (safewrite(logfile, timestamp, strlen(timestamp)) < 0 ||
-safewrite(logfile, START_POSTFIX, strlen(START_POSTFIX)) < 0) {
-VIR_WARN("Unable to write timestamp to logfile: %s",
- virStrerror(errno, ebuf, sizeof ebuf));
-}
-VIR_FREE(timestamp);
-
-/* Log generated command line */
-virCommandWriteArgLog(cmd, logfile);
-if ((pos = lseek(logfile, 0, SEEK_END)) < 0)
-VIR_WARN("Unable to seek to end of logfile: %s",
- virStrerror(errno, ebuf, sizeof ebuf));
-
virCommandPreserveFD(cmd, appPty);
virCommandSetOutputFD(cmd, &logfile);
virCommandSetErrorFD(cmd, &logfile);
-ret = virCommandRun(cmd, NULL);
-
+return cmd;
cleanup:
virCommandFree(cmd);
-return ret;
+return NULL;
}
@@ -1411,6 +1389,10 @@ static int lxcVmStart(virConnectPtr conn,
int logfd = -1;
unsigned int nveths = 0;
char **veths = NULL;
+off_t pos = -1;
+char ebuf[1024];
+char *timestamp;
+virCommandPtr cmd = NULL;
lxcDomainObjPrivatePtr priv = vm->privateData;
if (!lxc_driver->cgroup) {
@@ -1480,10 +1462,31 @@ static int lxcVmStart(virConnectPtr conn,
goto cleanup;
}
-if (lxcControllerStart(driver,
- vm,
- nveths, veths,
- parentTty, logfd) < 0)
+if (!(cmd = lxcBuildControllerCmd(driver,
+ vm,
+ nveths, veths,
+ parentTty, logfd)))
+goto cleanup;
+
+/* Log timestamp */
+if ((timestamp = virTimestamp()) == NULL) {
+virReportOOMError();
+goto cleanup;
+}
+if (safewrite(logfd, timestamp, strlen(timestamp)) < 0 ||
+safewrite(logfd, START_POSTFIX, strlen(START_POSTFIX)) < 0) {
+VIR_WARN("Unable to write timestamp to logfile: %s",
+ virStrerror(errno, ebuf, sizeof ebuf));
+}
+VIR_FREE(timestamp);
+
+/* Log generated command line */
+virCommandWriteArgLog(cmd, logfd);
+if ((pos = lseek(logfd, 0, SEEK_END)) < 0)
+VIR_WARN("Unable to seek to end of logfile: %s",
+ virStrerror(errno, ebuf, sizeof ebuf));
+
+if (virCommandRun(cmd, NULL) < 0)
goto cleanup;
/* Connect to the controller as a client *first* because
@@ -1529,6 +1532,7 @@ static int lxcVmStart(virConnectPtr conn,
rc = 0;
cleanup:
+virCommandFree(cmd);
if (VIR_CLOSE(logfd) < 0) {
virReportSystemError(errno, "%s", _("could not close logfile"));
rc = -1;
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/7] lxc: Don't report error in Wait/SendContinue
We will reuse these shortly, and each use should have a different error
message.
Signed-off-by: Cole Robinson
---
src/lxc/lxc_container.c | 15 ++-
src/lxc/lxc_controller.c |5 -
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 173af07..00add94 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -195,13 +195,10 @@ int lxcContainerSendContinue(int control)
writeCount = safewrite(control, &msg, sizeof(msg));
if (writeCount != sizeof(msg)) {
-virReportSystemError(errno, "%s",
- _("Unable to send container continue message"));
goto error_out;
}
rc = 0;
-
error_out:
return rc;
}
@@ -224,13 +221,8 @@ static int lxcContainerWaitForContinue(int control)
readLen = saferead(control, &msg, sizeof(msg));
if (readLen != sizeof(msg) ||
msg != LXC_CONTINUE_MSG) {
-virReportSystemError(errno, "%s",
- _("Failed to read the container continue
message"));
return -1;
}
-VIR_FORCE_CLOSE(control);
-
-VIR_DEBUG("Received container continue message");
return 0;
}
@@ -793,8 +785,12 @@ static int lxcContainerChild( void *data )
goto cleanup;
/* Wait for interface devices to show up */
-if (lxcContainerWaitForContinue(argv->monitor) < 0)
+if (lxcContainerWaitForContinue(argv->monitor) < 0) {
+virReportSystemError(errno, "%s",
+ _("Failed to read the container continue
message"));
goto cleanup;
+}
+VIR_DEBUG("Received container continue message");
/* rename and enable interfaces */
if (lxcContainerRenameAndEnableInterfaces(argv->nveths,
@@ -814,6 +810,7 @@ static int lxcContainerChild( void *data )
cleanup:
VIR_FREE(ttyPath);
VIR_FORCE_CLOSE(ttyfd);
+VIR_FORCE_CLOSE(argv->monitor);
if (ret == 0) {
/* this function will only return if an error occured */
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 5c4bd1f..cef4b58 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -731,8 +731,11 @@ lxcControllerRun(virDomainDefPtr def,
if (lxcControllerMoveInterfaces(nveths, veths, container) < 0)
goto cleanup;
-if (lxcContainerSendContinue(control[0]) < 0)
+if (lxcContainerSendContinue(control[0]) < 0) {
+virReportSystemError(errno, "%s",
+ _("Unable to send container continue message"));
goto cleanup;
+}
/* Now the container is running, there's no need for us to keep
any elevated capabilities */
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/7] lxc: Improve container startup error reporting
Currently most LXC container startup failures result in a reported success followed immediately by a guest shutoff. This patch series tries to improve the situation by adding a basic handshake from driver<->controller and controller<->container. If the handshake fails, the driver will return the output from the guest logfile, similar to how we do for qemu guests. A few additional error reporting improvements are also included. Cole Robinson (7): lxc: Drop container stdio as late as possible lxc: Don't report error in Wait/SendContinue lxc: Refactor controller command building lxc: Improve guest startup error reporting lxc: controller: Improve container error reporting lxc: Verify root fs exists before mounting lxc: Ensure container actually exists src/lxc/lxc_container.c | 67 +++--- src/lxc/lxc_container.h |2 + src/lxc/lxc_controller.c | 63 - src/lxc/lxc_driver.c | 173 +- 4 files changed, 243 insertions(+), 62 deletions(-) -- 1.7.4.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/7] lxc: controller: Improve container error reporting
Add a handshake with the cloned container process to try and detect
if it fails to start.
Signed-off-by: Cole Robinson
---
src/lxc/lxc_container.c | 18 ++
src/lxc/lxc_container.h |1 +
src/lxc/lxc_controller.c | 17 +
3 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index ff90842..26b493e 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -90,6 +90,7 @@ struct __lxc_child_argv {
char **veths;
int monitor;
char *ttyPath;
+int handshakefd;
};
@@ -128,7 +129,7 @@ static virCommandPtr
lxcContainerBuildInitCmd(virDomainDefPtr vmDef)
*
* Returns 0 on success or -1 in case of error
*/
-static int lxcContainerSetStdio(int control, int ttyfd)
+static int lxcContainerSetStdio(int control, int ttyfd, int handshakefd)
{
int rc = -1;
int open_max, i;
@@ -149,7 +150,7 @@ static int lxcContainerSetStdio(int control, int ttyfd)
* close all FDs before executing the container */
open_max = sysconf (_SC_OPEN_MAX);
for (i = 0; i < open_max; i++)
-if (i != ttyfd && i != control) {
+if (i != ttyfd && i != control && i != handshakefd) {
int tmpfd = i;
VIR_FORCE_CLOSE(tmpfd);
}
@@ -802,7 +803,13 @@ static int lxcContainerChild( void *data )
if (lxcContainerDropCapabilities() < 0)
goto cleanup;
-if (lxcContainerSetStdio(argv->monitor, ttyfd) < 0) {
+if (lxcContainerSendContinue(argv->handshakefd) < 0) {
+virReportSystemError(errno, "%s",
+_("failed to send continue signal to controller"));
+goto cleanup;
+}
+
+if (lxcContainerSetStdio(argv->monitor, ttyfd, argv->handshakefd) < 0) {
goto cleanup;
}
@@ -811,6 +818,7 @@ cleanup:
VIR_FREE(ttyPath);
VIR_FORCE_CLOSE(ttyfd);
VIR_FORCE_CLOSE(argv->monitor);
+VIR_FORCE_CLOSE(argv->handshakefd);
if (ret == 0) {
/* this function will only return if an error occured */
@@ -870,13 +878,15 @@ int lxcContainerStart(virDomainDefPtr def,
unsigned int nveths,
char **veths,
int control,
+ int handshakefd,
char *ttyPath)
{
pid_t pid;
int flags;
int stacksize = getpagesize() * 4;
char *stack, *stacktop;
-lxc_child_argv_t args = { def, nveths, veths, control, ttyPath };
+lxc_child_argv_t args = { def, nveths, veths, control, ttyPath,
+ handshakefd};
/* allocate a stack for the container */
if (VIR_ALLOC_N(stack, stacksize) < 0) {
diff --git a/src/lxc/lxc_container.h b/src/lxc/lxc_container.h
index a3e457e..d6d9b6d 100644
--- a/src/lxc/lxc_container.h
+++ b/src/lxc/lxc_container.h
@@ -52,6 +52,7 @@ int lxcContainerStart(virDomainDefPtr def,
unsigned int nveths,
char **veths,
int control,
+ int handshakefd,
char *ttyPath);
int lxcContainerAvailable(int features);
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 5bf8ee3..c94d0d0 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -617,6 +617,7 @@ lxcControllerRun(virDomainDefPtr def,
{
int rc = -1;
int control[2] = { -1, -1};
+int containerhandshake[2] = { -1, -1 };
int containerPty = -1;
char *containerPtyPath = NULL;
pid_t container = -1;
@@ -630,6 +631,12 @@ lxcControllerRun(virDomainDefPtr def,
goto cleanup;
}
+if (socketpair(PF_UNIX, SOCK_STREAM, 0, containerhandshake) < 0) {
+virReportSystemError(errno, "%s",
+ _("socketpair failed"));
+goto cleanup;
+}
+
root = virDomainGetRootFilesystem(def);
if (lxcSetContainerResources(def) < 0)
@@ -725,9 +732,11 @@ lxcControllerRun(virDomainDefPtr def,
nveths,
veths,
control[1],
+ containerhandshake[1],
containerPtyPath)) < 0)
goto cleanup;
VIR_FORCE_CLOSE(control[1]);
+VIR_FORCE_CLOSE(containerhandshake[1]);
if (lxcControllerMoveInterfaces(nveths, veths, container) < 0)
goto cleanup;
@@ -738,6 +747,12 @@ lxcControllerRun(virDomainDefPtr def,
goto cleanup;
}
+if (lxcContainerWaitForContinue(containerhandshake[0]) < 0) {
+virReportSystemError(errno, "%s",
+ _("error receiving signal from container"));
+goto cleanup;
+}
+
/* Now the container is running, there's no need for us to keep
any elevated capabilities */
if (lxcControllerClearCapabilities() < 0)
@@ -760,6 +775,8 @@ cleanup:
VIR_FREE(con
Re: [libvirt] [PATCH] Fix regressions BlockStats/Info APIs in QEMU driver
On 06/02/2011 10:03 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange"
>
> The change 18c2a592064d69499f70428e498f4a3cb5161cda caused
> some regressions in behaviour of virDomainBlockStats
> and virDomainBlockInfo in the QEMU driver.
>
> The virDomainBlockInfo API stopped working for inactive
> guests if querying a block device.
>
> The virDomainBlockStats API did not promptly report
> an error if the guest was not running in some cases.
>
> * src/qemu/qemu_driver.c: Fix inactive guest handling
> in BlockStats/Info APIs
> ---
> src/qemu/qemu_driver.c | 17 +++--
> 1 files changed, 11 insertions(+), 6 deletions(-)
>
> @@ -6017,10 +6024,9 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,
> if (qemuDomainObjBeginJob(vm) < 0)
> goto cleanup;
>
> -if (!virDomainObjIsActive(vm)) {
> -qemuReportError(VIR_ERR_OPERATION_INVALID,
> -"%s", _("domain is not running"));
> -goto endjob;
> +if (virDomainObjIsActive(vm)) {
> +ret = 0;
> +goto cleanup;
Oops - you lost the ! in that conditional. Also, 'goto cleanup' forgets
to end the job condition that we hold. The real answer is that if the
domain is not active, we set ret to 0 and short-circuit the attempt to
query the guest.
Conditional ACK if you change this hunk to be:
if (!virDomainObjIsActive(vm)) {
ret = 0;
if (qemuDomainObjEndJob(vm) == 0)
vm = NULL;
goto cleanup;
}
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Explicitly set VM state to paused after migration completes
On 06/02/2011 09:42 AM, Daniel P. Berrange wrote: > In v3 migration, once migration is completed, the VM needs > to be left in a paused state until after Finish3 has been > executed on the target. Only then will the VM be killed > off. When using non-JSON QEMU monitor though, we don't > receive any 'STOP' event from QEMU, so we need to manually > set our state offline & thus release lock manager leases. > It doesn't hurt to run this on the JSON case too, just in > case the event gets lost somehow > > * src/qemu/qemu_migration.c: Explicitly set VM state to > paused when migration completes > --- > src/qemu/qemu_migration.c | 28 +++- > 1 files changed, 27 insertions(+), 1 deletions(-) ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] to correct UML command line networking parameters
I have been finding that some UML command line networking parameters are
being generated incorrectly. Attached is a patch that corrects the problem.
For more information, see
https://bugzilla.redhat.com/show_bug.cgi?id=706295 .
diff -Naur libvirt-0.9.1.orig/src/uml/uml_conf.c libvirt-0.9.1.new/src/uml/uml_conf.c
--- libvirt-0.9.1.orig/src/uml/uml_conf.c 2011-03-01 01:03:32.0 -0600
+++ libvirt-0.9.1.new/src/uml/uml_conf.c 2011-05-27 20:27:26.563772975 -0500
@@ -208,6 +208,11 @@
case VIR_DOMAIN_NET_TYPE_ETHERNET:
/* ethNNN=tuntap,tapname,macaddr,gateway */
virBufferAddLit(&buf, "tuntap");
+if (def->ifname) {
+virBufferVSprintf(&buf, ",%s", def->ifname);
+} else {
+virBufferAddLit(&buf, ",");
+}
if (def->data.ethernet.ipaddr) {
umlReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("IP address not supported for ethernet inteface"));
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix regressions BlockStats/Info APIs in QEMU driver
From: "Daniel P. Berrange"
The change 18c2a592064d69499f70428e498f4a3cb5161cda caused
some regressions in behaviour of virDomainBlockStats
and virDomainBlockInfo in the QEMU driver.
The virDomainBlockInfo API stopped working for inactive
guests if querying a block device.
The virDomainBlockStats API did not promptly report
an error if the guest was not running in some cases.
* src/qemu/qemu_driver.c: Fix inactive guest handling
in BlockStats/Info APIs
---
src/qemu/qemu_driver.c | 17 +++--
1 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5632d62..3716b15 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5531,6 +5531,12 @@ qemudDomainBlockStats (virDomainPtr dom,
goto cleanup;
}
+if (!virDomainObjIsActive(vm)) {
+qemuReportError(VIR_ERR_OPERATION_INVALID,
+"%s", _("domain is not running"));
+goto cleanup;
+}
+
for (i = 0 ; i < vm->def->ndisks ; i++) {
if (STREQ(path, vm->def->disks[i]->dst)) {
disk = vm->def->disks[i];
@@ -5994,7 +6000,8 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,
highest allocated extent from QEMU */
if (disk->type == VIR_DOMAIN_DISK_TYPE_BLOCK &&
format != VIR_STORAGE_FILE_RAW &&
-S_ISBLK(sb.st_mode)) {
+S_ISBLK(sb.st_mode) &&
+virDomainObjIsActive(vm)) {
qemuDomainObjPrivatePtr priv = vm->privateData;
if ((priv->jobActive == QEMU_JOB_MIGRATION_OUT)
@@ -6017,10 +6024,9 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,
if (qemuDomainObjBeginJob(vm) < 0)
goto cleanup;
-if (!virDomainObjIsActive(vm)) {
-qemuReportError(VIR_ERR_OPERATION_INVALID,
-"%s", _("domain is not running"));
-goto endjob;
+if (virDomainObjIsActive(vm)) {
+ret = 0;
+goto cleanup;
}
qemuDomainObjEnterMonitor(vm);
@@ -6029,7 +6035,6 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,
&info->allocation);
qemuDomainObjExitMonitor(vm);
-endjob:
if (qemuDomainObjEndJob(vm) == 0)
vm = NULL;
}
--
1.7.5.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix error reporting with virDomainBlockStats on inactive guest
On Thu, Jun 02, 2011 at 08:26:50AM -0600, Eric Blake wrote: > On 06/02/2011 07:58 AM, Daniel P. Berrange wrote: > > From: "Daniel P. Berrange" > > > > Pull the check on virDomainIsActive upto top of the QEMU > > s/upto/up to/ > > > impl of virDomainBlockStats so that is is reported promptly > > > > * src/qemu/qemu_driver.c: Check if guest is active immediately > > --- > > src/qemu/qemu_driver.c |6 ++ > > 1 files changed, 6 insertions(+), 0 deletions(-) > > This means that we are now checking multiple times (once on entry for > immediate effect even if migration is under way, and again after > acquiring job condition if migration is not under way), but that > shouldn't hurt. > > However, you also need to do the same thing for qemuDomainGetBlockInfo, > since both methods were given the same control flow during commit 18c2a592. qemuDomainGetBlockInfo is actually worse. Previously qemuDomainGetBlockInfo worked just fine on inactive guests, but now it will throw an error :-( Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] docs: document iface-* commands
On 05/31/2011 04:43 PM, Eric Blake wrote:
I intentionally set things up so 'virsh help interface' lists
commands in alphabetical order, but 'man virsh' lists them in
topical order; this matches our practice on some other commands.
* tools/virsh.pod: Document all iface commands.
* tools/virsh.c (ifaceCmds): Sort.
---
v2: fix points raised by Laine.
Interdiff from v1 before the actual patch:
=head1 INTERFACE COMMANDS
-The following commands manipulate host interfaces to be used by
-virtual networks. A lot of the commands for host interfaces are
-similar to the ones used for domains, and the way to name an interface
-is either by its name or its MAC address.
+The following commands manipulate host interfaces. Often, these host
+interfaces can then be used by name within domain elements
+(such as a system-created bridge interface), but there is no
+requirement that host interfaces be tied to any particular guest
+configuration XML at all.
+
+A lot of the commands for host interfaces are similar to the ones used
+for domains, and the way to name an interface is either by its name or
+its MAC address. However, using a MAC address for an I
+argument only works when that address is unique (if an interface and a
+bridge share the same MAC address, which is often the case, then using
+that MAC address results in an error due to ambiguity, and you must
+resort to a name instead).
=over 4
@@ -985,7 +994,8 @@ I<--inactive> is specified only the inactive ones will be
listed.
=item B I
-Convert a host interface MAC to interface name.
+Convert a host interface MAC to interface name, if the I is
+unique among the host's interfaces.
=item B I
@@ -993,7 +1003,7 @@ Convert a host interface name to MAC address.
=item B I
-Start a (previously defined) host interface.
+Start a (previously defined) host interface, such as by running "if-up".
=item B I
@@ -1011,17 +1021,16 @@ commit or rollback.
=item B
-Confirm that all changes since the last I are working,
-and delete the rollback point. If no interface snapshot has already
-been started, then this command will fail.
+Declare all changes since the last I as working, and
+delete the rollback point. If no interface snapshot has already been
+started, then this command will fail.
=item B
Revert all host interface settings back to the state recorded in the
last I. If no interface snapshot has already been
-started, then this command will fail. If appropriate netcf init
-scripts are also installed, then rebooting the host may serve as an
-implicit rollback point.
+started, then this command will fail. Rebooting the host also serves
+as an implicit rollback point.
=back
tools/virsh.c | 12 +++---
tools/virsh.pod | 99 ++-
2 files changed, 104 insertions(+), 7 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index dfd5bd2..b9f1101 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -11063,6 +11063,10 @@ static const vshCmdDef nodedevCmds[] = {
};
static const vshCmdDef ifaceCmds[] = {
+{"iface-begin", cmdInterfaceBegin, opts_interface_begin,
+ info_interface_begin, 0},
+{"iface-commit", cmdInterfaceCommit, opts_interface_commit,
+ info_interface_commit, 0},
{"iface-define", cmdInterfaceDefine, opts_interface_define,
info_interface_define, 0},
{"iface-destroy", cmdInterfaceDestroy, opts_interface_destroy,
@@ -11077,16 +11081,12 @@ static const vshCmdDef ifaceCmds[] = {
info_interface_mac, 0},
{"iface-name", cmdInterfaceName, opts_interface_name,
info_interface_name, 0},
+{"iface-rollback", cmdInterfaceRollback, opts_interface_rollback,
+ info_interface_rollback, 0},
{"iface-start", cmdInterfaceStart, opts_interface_start,
info_interface_start, 0},
{"iface-undefine", cmdInterfaceUndefine, opts_interface_undefine,
info_interface_undefine, 0},
-{"iface-begin", cmdInterfaceBegin, opts_interface_begin,
- info_interface_begin, 0},
-{"iface-commit", cmdInterfaceCommit, opts_interface_commit,
- info_interface_commit, 0},
-{"iface-rollback", cmdInterfaceRollback, opts_interface_rollback,
- info_interface_rollback, 0},
{NULL, NULL, NULL, NULL, 0}
};
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 9251db6..ae0c887 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -865,7 +865,7 @@ The following commands manipulate networks. Libvirt has the
capability to
define virtual networks which can then be used by domains and linked to
actual network devices. For more detailed information about this feature
see the documentation at L . A lot
-of the command for virtual networks are similar to the one used for domains,
+of the commands for virtual networks are similar to the ones used for domains,
As long as you're fixing this typo, how about changing "A lot" (which I
consider too informal for wr
[libvirt] [PATCH] Explicitly set VM state to paused after migration completes
In v3 migration, once migration is completed, the VM needs
to be left in a paused state until after Finish3 has been
executed on the target. Only then will the VM be killed
off. When using non-JSON QEMU monitor though, we don't
receive any 'STOP' event from QEMU, so we need to manually
set our state offline & thus release lock manager leases.
It doesn't hurt to run this on the JSON case too, just in
case the event gets lost somehow
* src/qemu/qemu_migration.c: Explicitly set VM state to
paused when migration completes
---
src/qemu/qemu_migration.c | 28 +++-
1 files changed, 27 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index f4eda92..aa74d86 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -718,7 +718,7 @@ qemuMigrationSetOffline(struct qemud_driver *driver,
virDomainObjPtr vm)
{
int ret;
-
+VIR_DEBUG("driver=%p vm=%p", driver, vm);
ret = qemuProcessStopCPUs(driver, vm, VIR_DOMAIN_PAUSED_MIGRATION);
if (ret == 0) {
virDomainEventPtr event;
@@ -1521,6 +1521,19 @@ static int doNativeMigrate(struct qemud_driver *driver,
if (qemuMigrationWaitForCompletion(driver, vm) < 0)
goto cleanup;
+/* When migration completed, QEMU will have paused the
+ * CPUs for us, but unless we're using the JSON monitor
+ * we won't have been notified of this, so might still
+ * think we're running. For v2 protocol this doesn't
+ * matter because we'll kill the VM soon, but for v3
+ * this is important because we stay paused until the
+ * confirm3 step, but need to release the lock state
+ */
+if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
+if (qemuMigrationSetOffline(driver, vm) < 0)
+goto cleanup;
+}
+
if (qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen, 0) <
0)
VIR_WARN("Unable to encode migration cookie");
@@ -1816,6 +1829,19 @@ static int doTunnelMigrate(struct qemud_driver *driver,
ret = qemuMigrationWaitForCompletion(driver, vm);
+/* When migration completed, QEMU will have paused the
+ * CPUs for us, but unless we're using the JSON monitor
+ * we won't have been notified of this, so might still
+ * think we're running. For v2 protocol this doesn't
+ * matter because we'll kill the VM soon, but for v3
+ * this is important because we stay paused until the
+ * confirm3 step, but need to release the lock state
+ */
+if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) {
+if (qemuMigrationSetOffline(driver, vm) < 0)
+goto cleanup;
+}
+
/* Close now to ensure the IO thread quits & is joinable in next method */
VIR_FORCE_CLOSE(client_sock);
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Building on Solaris 11 Express
Hi Matthias, On Sun, May 29, 2011 at 12:34, Matthias Bolte wrote: > That's strange, configure should automatically disable the Xen driver > if it can't find the required headers and libs. The only way I can see > from the logic in configure is that configure found libxenstore but > can't find the headers such as xen/xen.h xen/version.h xen/dom0_ops.h > xen/sys/privcmd.h xen/linux/privcmd.h. I think you're right. In my default OpenIndiana b151 install, /usr/lib/xenstore.so exists (from the xvmstore package), but not the xvm headers. They're in the xvm-headers package. I assume it's the same on Solaris Express 11. Kind regards, Ruben -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix error reporting with virDomainBlockStats on inactive guest
On 06/02/2011 07:58 AM, Daniel P. Berrange wrote: > From: "Daniel P. Berrange" > > Pull the check on virDomainIsActive upto top of the QEMU s/upto/up to/ > impl of virDomainBlockStats so that is is reported promptly > > * src/qemu/qemu_driver.c: Check if guest is active immediately > --- > src/qemu/qemu_driver.c |6 ++ > 1 files changed, 6 insertions(+), 0 deletions(-) This means that we are now checking multiple times (once on entry for immediate effect even if migration is under way, and again after acquiring job condition if migration is not under way), but that shouldn't hurt. However, you also need to do the same thing for qemuDomainGetBlockInfo, since both methods were given the same control flow during commit 18c2a592. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix auditing of disk hotunplug operations
On 06/02/2011 07:58 AM, Daniel P. Berrange wrote: > From: "Daniel P. Berrange" > > The qemuAuditDisk calls in disk hotunplug operations were being > passed 'ret >= 0', but the code which sets ret to 0 was not yet > executed, and the error path had already jumped to the 'cleanup' > label. This meant hotunplug failures were never audited, and > hotunplug success was audited as a failure > > * src/qemu/qemu_hotplug.c: Fix auditing of hotunplug > --- > src/qemu/qemu_hotplug.c |7 +-- > 1 files changed, 5 insertions(+), 2 deletions(-) ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Avoid crash on NULL pointer in lock driver impls during hotplug
On 06/02/2011 07:58 AM, Daniel P. Berrange wrote: > From: "Daniel P. Berrange" > > When virLockDriverAcquire is invoked during hotplug the state > parameter will be left as NULL. > > * src/locking/lock_driver_nop.c, > src/locking/lock_driver_sanlock.c: Don't reference NULL state > parameter > --- > src/locking/lock_driver_nop.c |7 --- > src/locking/lock_driver_sanlock.c | 29 ++--- > 2 files changed, 22 insertions(+), 14 deletions(-) ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Libvirt 0.9.2 week freeze, RC2 available
Hi Eric, On Thu, Jun 2, 2011 at 16:06, Eric Blake wrote: > Why were you using --with-openvz in the first place? You have to ask Justin, I used his Homebrew formula :-). It's always been in there I think. > Does OpenVZ work > on OSX, in which case commit e85a602b represents a regression for > restricting it to Linux-only? Or did you have a typo, where you meant > that you previously had to use --without-openvz, and now you no longer > need to because the configure.ac patch did the right thing? No, e85a602b does the right thing. Justin is going to update the formula. Kind regards, Ruben -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Make sure virDomainSave/virDomainManagedSave reset id to -1
On 06/02/2011 07:57 AM, Daniel P. Berrange wrote: > From: "Daniel P. Berrange" > > After successfull virDomainSave/virDomainManagedSave calls > the guest will no longer be active, so the domain ID must > be reset to -1 > > * daemon/remote_generator.pl: Special case virDomainSave & > virDomainManagedSave for same reason as virDomainDestroy > --- > daemon/remote_generator.pl |7 +-- > 1 files changed, 5 insertions(+), 2 deletions(-) ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix return value in lock manager hotplug methods
On 06/02/2011 07:57 AM, Daniel P. Berrange wrote: > From: "Daniel P. Berrange" > > Refactoring of the lock manager hotplug methods lost the > ret = 0 assignment for successful return path > > * src/locking/domain_lock.c: Add missing ret = 0 assignments > --- > src/locking/domain_lock.c |8 > 1 files changed, 8 insertions(+), 0 deletions(-) ACK. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Libvirt 0.9.2 week freeze, RC2 available
On 06/02/2011 07:45 AM, Ruben Kerkhof wrote: >> If people could also try -rc2 it would be great too, >> >> thanks ! >> >> Daniel > > Still works on OSX. > This time I had to remove the --with-openvz option from the Homebrew formula. Why were you using --with-openvz in the first place? Does OpenVZ work on OSX, in which case commit e85a602b represents a regression for restricting it to Linux-only? Or did you have a typo, where you meant that you previously had to use --without-openvz, and now you no longer need to because the configure.ac patch did the right thing? -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: avoid corrupting / in RHEL 5
On 06/02/2011 06:35 AM, Daniel P. Berrange wrote: > On Thu, Jun 02, 2011 at 05:48:39AM -0600, Eric Blake wrote: >> On 06/02/2011 05:26 AM, Daniel P. Berrange wrote: >>> >>> I've seen a strange build error from autobuild.sh which I'm thinking >>> may be related to this patch >>> >>> Generating drivers.html.tmp >>> Stubbing todo.html.in >>> Generating internals/locking.html.tmp >>> Generating internals/command.html.tmp >> >> There's no locking.html.in yet (except in your patch series), so this >> points to a bug in at least the VPATH rules for your conversion from >> locking.html.in to locking.html. >> >> You need to generate locking.html in $(srcdir), since it will be >> distributed pre-built as part of the tarball. > > There's no special makefile rules for locking.html - all the > HTML stuff is wildcarded. The same error hits command.html > here too. Ah, I see the problem now. locking.html is generated in $(srcdir), but it goes via an intermediate locking.html.tmp, which is indeed generated in $(builddir). I didn't catch it earlier, because I was testing an in-tree build (where the builddir happens to already exist, since it is also the srcdir). I'll push the obvious fix (restoring the MKDIR_P, but without the problematic $(builddir) that RHEL 5 didn't like). -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix error reporting with virDomainBlockStats on inactive guest
From: "Daniel P. Berrange"
Pull the check on virDomainIsActive upto top of the QEMU
impl of virDomainBlockStats so that is is reported promptly
* src/qemu/qemu_driver.c: Check if guest is active immediately
---
src/qemu/qemu_driver.c |6 ++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5632d62..3f8f814 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5531,6 +5531,12 @@ qemudDomainBlockStats (virDomainPtr dom,
goto cleanup;
}
+if (!virDomainObjIsActive(vm)) {
+qemuReportError(VIR_ERR_OPERATION_INVALID,
+"%s", _("domain is not running"));
+goto cleanup;
+}
+
for (i = 0 ; i < vm->def->ndisks ; i++) {
if (STREQ(path, vm->def->disks[i]->dst)) {
disk = vm->def->disks[i];
--
1.7.5.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix auditing of disk hotunplug operations
From: "Daniel P. Berrange"
The qemuAuditDisk calls in disk hotunplug operations were being
passed 'ret >= 0', but the code which sets ret to 0 was not yet
executed, and the error path had already jumped to the 'cleanup'
label. This meant hotunplug failures were never audited, and
hotunplug success was audited as a failure
* src/qemu/qemu_hotplug.c: Fix auditing of hotunplug
---
src/qemu/qemu_hotplug.c |7 +--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index c9e2d08..fe47896 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1191,12 +1191,14 @@ int qemuDomainDetachPciDiskDevice(struct qemud_driver
*driver,
if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) {
qemuDomainObjExitMonitor(vm);
+qemuAuditDisk(vm, detach, NULL, "detach", false);
goto cleanup;
}
} else {
if (qemuMonitorRemovePCIDevice(priv->mon,
&detach->info.addr.pci) < 0) {
qemuDomainObjExitMonitor(vm);
+qemuAuditDisk(vm, detach, NULL, "detach", false);
goto cleanup;
}
}
@@ -1206,7 +1208,7 @@ int qemuDomainDetachPciDiskDevice(struct qemud_driver
*driver,
qemuDomainObjExitMonitorWithDriver(driver, vm);
-qemuAuditDisk(vm, detach, NULL, "detach", ret >= 0);
+qemuAuditDisk(vm, detach, NULL, "detach", true);
if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) &&
qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &detach->info) < 0)
@@ -1283,6 +1285,7 @@ int qemuDomainDetachDiskDevice(struct qemud_driver
*driver,
qemuDomainObjEnterMonitorWithDriver(driver, vm);
if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) {
qemuDomainObjExitMonitor(vm);
+qemuAuditDisk(vm, detach, NULL, "detach", false);
goto cleanup;
}
@@ -1291,7 +1294,7 @@ int qemuDomainDetachDiskDevice(struct qemud_driver
*driver,
qemuDomainObjExitMonitorWithDriver(driver, vm);
-qemuAuditDisk(vm, detach, NULL, "detach", ret >= 0);
+qemuAuditDisk(vm, detach, NULL, "detach", true);
virDomainDiskRemove(vm->def, i);
--
1.7.5.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Avoid crash on NULL pointer in lock driver impls during hotplug
From: "Daniel P. Berrange"
When virLockDriverAcquire is invoked during hotplug the state
parameter will be left as NULL.
* src/locking/lock_driver_nop.c,
src/locking/lock_driver_sanlock.c: Don't reference NULL state
parameter
---
src/locking/lock_driver_nop.c |7 ---
src/locking/lock_driver_sanlock.c | 29 ++---
2 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/src/locking/lock_driver_nop.c b/src/locking/lock_driver_nop.c
index 5ebbd8d..36a9083 100644
--- a/src/locking/lock_driver_nop.c
+++ b/src/locking/lock_driver_nop.c
@@ -76,7 +76,8 @@ static int virLockManagerNopRelease(virLockManagerPtr lock
ATTRIBUTE_UNUSED,
char **state,
unsigned int flags ATTRIBUTE_UNUSED)
{
-*state = NULL;
+if (state)
+*state = NULL;
return 0;
}
@@ -85,8 +86,8 @@ static int virLockManagerNopInquire(virLockManagerPtr lock
ATTRIBUTE_UNUSED,
char **state,
unsigned int flags ATTRIBUTE_UNUSED)
{
-
-*state = NULL;
+if (state)
+*state = NULL;
return 0;
}
diff --git a/src/locking/lock_driver_sanlock.c
b/src/locking/lock_driver_sanlock.c
index a60d7ce..adead76 100644
--- a/src/locking/lock_driver_sanlock.c
+++ b/src/locking/lock_driver_sanlock.c
@@ -374,18 +374,20 @@ static int virLockManagerSanlockRelease(virLockManagerPtr
lock,
virCheckFlags(0, -1);
-if ((rv = sanlock_inquire(-1, priv->vm_pid, 0, &res_count, state)) < 0) {
-if (rv <= -200)
-virLockError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to inquire lock: error %d"), rv);
-else
-virReportSystemError(-rv, "%s",
- _("Failed to inquire lock"));
-return -1;
-}
+if (state) {
+if ((rv = sanlock_inquire(-1, priv->vm_pid, 0, &res_count, state)) <
0) {
+if (rv <= -200)
+virLockError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to inquire lock: error %d"), rv);
+else
+virReportSystemError(-rv, "%s",
+ _("Failed to inquire lock"));
+return -1;
+}
-if (STREQ(*state, ""))
-VIR_FREE(*state);
+if (STREQ(*state, ""))
+VIR_FREE(*state);
+}
if ((rv = sanlock_release(-1, priv->vm_pid, SANLK_REL_ALL, 0, NULL)) < 0) {
if (rv <= -200)
@@ -409,6 +411,11 @@ static int virLockManagerSanlockInquire(virLockManagerPtr
lock,
virCheckFlags(0, -1);
+if (!state) {
+virLockError(VIR_ERR_INVALID_ARG, "state");
+return -1;
+}
+
VIR_DEBUG("pid=%d", priv->vm_pid);
if ((rv = sanlock_inquire(-1, priv->vm_pid, 0, &res_count, state)) < 0) {
--
1.7.5.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix return value in lock manager hotplug methods
From: "Daniel P. Berrange" Refactoring of the lock manager hotplug methods lost the ret = 0 assignment for successful return path * src/locking/domain_lock.c: Add missing ret = 0 assignments --- src/locking/domain_lock.c |8 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/src/locking/domain_lock.c b/src/locking/domain_lock.c index f0a11b7..771ed53 100644 --- a/src/locking/domain_lock.c +++ b/src/locking/domain_lock.c @@ -221,6 +221,8 @@ int virDomainLockDiskAttach(virLockManagerPluginPtr plugin, if (virLockManagerAcquire(lock, NULL, 0) < 0) goto cleanup; +ret = 0; + cleanup: virLockManagerFree(lock); @@ -240,6 +242,8 @@ int virDomainLockDiskDetach(virLockManagerPluginPtr plugin, if (virLockManagerRelease(lock, NULL, 0) < 0) goto cleanup; +ret = 0; + cleanup: virLockManagerFree(lock); @@ -260,6 +264,8 @@ int virDomainLockLeaseAttach(virLockManagerPluginPtr plugin, if (virLockManagerAcquire(lock, NULL, 0) < 0) goto cleanup; +ret = 0; + cleanup: virLockManagerFree(lock); @@ -279,6 +285,8 @@ int virDomainLockLeaseDetach(virLockManagerPluginPtr plugin, if (virLockManagerRelease(lock, NULL, 0) < 0) goto cleanup; +ret = 0; + cleanup: virLockManagerFree(lock); -- 1.7.5.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Make sure virDomainSave/virDomainManagedSave reset id to -1
From: "Daniel P. Berrange"
After successfull virDomainSave/virDomainManagedSave calls
the guest will no longer be active, so the domain ID must
be reset to -1
* daemon/remote_generator.pl: Special case virDomainSave &
virDomainManagedSave for same reason as virDomainDestroy
---
daemon/remote_generator.pl |7 +--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/daemon/remote_generator.pl b/daemon/remote_generator.pl
index e204b76..6bddfa9 100755
--- a/daemon/remote_generator.pl
+++ b/daemon/remote_generator.pl
@@ -1363,8 +1363,11 @@ elsif ($opt_k) {
print "\n";
}
-if ($call->{ProcName} eq "DomainDestroy") {
-# SPECIAL: virDomainDestroy needs to reset the domain id explicitly
+if ($call->{ProcName} eq "DomainDestroy" ||
+ $call->{ProcName} eq "DomainSave" ||
+ $call->{ProcName} eq "DomainManagedSave") {
+# SPECIAL: virDomain{Destroy|Save|ManagedSave} need to reset
+ # the domain id explicitly on success
print "dom->id = -1;\n";
}
--
1.7.5.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Libvirt 0.9.2 week freeze, RC2 available
On Thu, Jun 2, 2011 at 15:05, Daniel Veillard wrote: > I just pushed a second release candidate > ftp://libvirt.org/libvirt/libvirt-0.9.2-rc2.tar.gz > with corresponding rpm builds, this includes the extrenal lock support. > > On Wed, Jun 01, 2011 at 12:49:52PM +0200, Ruben Kerkhof wrote: >> On Wed, Jun 1, 2011 at 10:31, Matthias Bolte >> wrote: >> > Works on Ubuntu 10.04, Windows (MinGW) and FreeBSD. >> > >> > Matthias >> >> Works on OSX 10.7.0, tested with VirtualBox 4.08 > > Excellent news, thanks ! > I hope we didn't introduced regressions in portability with the latest > API additions ! If people could also try -rc2 it would be great too, > > thanks ! > > Daniel Still works on OSX. This time I had to remove the --with-openvz option from the Homebrew formula. Ruben -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Libvirt 0.9.2 week freeze, RC2 available
I just pushed a second release candidate ftp://libvirt.org/libvirt/libvirt-0.9.2-rc2.tar.gz with corresponding rpm builds, this includes the extrenal lock support. On Wed, Jun 01, 2011 at 12:49:52PM +0200, Ruben Kerkhof wrote: > On Wed, Jun 1, 2011 at 10:31, Matthias Bolte > wrote: > > Works on Ubuntu 10.04, Windows (MinGW) and FreeBSD. > > > > Matthias > > Works on OSX 10.7.0, tested with VirtualBox 4.08 Excellent news, thanks ! I hope we didn't introduced regressions in portability with the latest API additions ! If people could also try -rc2 it would be great too, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] screenshot: Expose the new API in virsh
On 02.06.2011 14:55, Daniel Veillard wrote:
> On Thu, Jun 02, 2011 at 01:26:22PM +0200, Michal Privoznik wrote:
>> * tools/virsh.c: Add screenshot command
>> * tools/virsh.pod: Document new command
>> * src/libvirt.c: Fix off-be-one error
>> ---
>> diff to v1:
>> - make filename optional and generate filename when missing
>>
>> diff to v2:
>> - Eric's review suggestions included
>>
>> src/libvirt.c |2 +-
>> tools/virsh.c | 187
>> +++---
>> tools/virsh.pod |9 +++
>> 3 files changed, 186 insertions(+), 12 deletions(-)
>>
>> diff --git a/src/libvirt.c b/src/libvirt.c
>> index ee5c7cd..eaae0ec 100644
>> --- a/src/libvirt.c
>> +++ b/src/libvirt.c
>> @@ -2464,7 +2464,7 @@ error:
>> * The screen ID is the sequential number of screen. In case of multiple
>> * graphics cards, heads are enumerated before devices, e.g. having
>> * two graphics cards, both with four heads, screen ID 5 addresses
>> - * the first head on the second card.
>> + * the second head on the second card.
>> *
>> * Returns a string representing the mime-type of the image format, or
>> * NULL upon error. The caller must free() the returned value.
>> diff --git a/tools/virsh.c b/tools/virsh.c
>> index dfd5bd2..da10a0b 100644
>> --- a/tools/virsh.c
>> +++ b/tools/virsh.c
>> @@ -264,6 +264,9 @@ static bool vshCmdGrpHelp(vshControl *ctl, const char
>> *name);
>> static vshCmdOpt *vshCommandOpt(const vshCmd *cmd, const char *name);
>> static int vshCommandOptInt(const vshCmd *cmd, const char *name, int *value)
>> ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
>> +static int vshCommandOptUInt(const vshCmd *cmd, const char *name,
>> + unsigned int *value)
>> +ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
>> static int vshCommandOptUL(const vshCmd *cmd, const char *name,
>> unsigned long *value)
>> ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
>> @@ -1938,6 +1941,153 @@ cmdDump(vshControl *ctl, const vshCmd *cmd)
>> return ret;
>> }
>>
>> +static const vshCmdInfo info_screenshot[] = {
>> +{"help", N_("take a screenshot of a current domain console and store it
>> "
>> +"into a file")},
>> +{"desc", N_("screenshot of a current domain console")},
>> +{NULL, NULL}
>> +};
>> +
>> +static const vshCmdOptDef opts_screenshot[] = {
>> +{"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
>> +{"file", VSH_OT_DATA, VSH_OFLAG_NONE, N_("where to store the
>> screenshot")},
>> +{"screen", VSH_OT_INT, VSH_OFLAG_NONE, N_("ID of a screen to take
>> screenshot of")},
>> +{NULL, 0, 0, NULL}
>> +};
>> +
>> +static int vshStreamSink(virStreamPtr st ATTRIBUTE_UNUSED,
>> + const char *bytes, size_t nbytes, void *opaque)
>> +{
>> +int *fd = opaque;
>> +
>> +return safewrite(*fd, bytes, nbytes);
>> +}
>> +
>> +/**
>> + * Generate string: '-[]'
>> + */
>> +static char *
>> +vshGenFileName(vshControl *ctl, virDomainPtr dom, const char *mime)
>> +{
>> +char timestr[100];
>> +struct timeval cur_time;
>> +struct tm time_info;
>> +const char *ext = NULL;
>> +char *ret = NULL;
>> +
>> +/* We should be already connected, but doesn't
>> + * hurt to check */
>> +if (!vshConnectionUsability(ctl, ctl->conn))
>> +return NULL;
>> +
>> +if (!dom) {
>> +vshError(ctl, "%s", _("Invalid domain supplied"));
>> +return NULL;
>> +}
>> +
>> +if (STREQ(mime, "image/x-portable-pixmap"))
>> +ext = ".ppm";
>> +else if (STREQ(mime, "image/png"))
>> +ext = ".png";
>> +/* add mime type here */
>> +
>> +gettimeofday(&cur_time, NULL);
>> +localtime_r(&cur_time.tv_sec, &time_info);
>> +strftime(timestr, sizeof(timestr), "%Y-%m-%d-%H:%M:%S", &time_info);
>> +
>> +if (virAsprintf(&ret, "%s-%s%s", virDomainGetName(dom),
>> +timestr, ext ? ext : "") < 0) {
>> +vshError(ctl, "%s", _("Out of memory"));
>> +return NULL;
>> +}
>> +
>> +return ret;
>> +}
>> +
>> +static bool
>> +cmdScreenshot(vshControl *ctl, const vshCmd *cmd)
>> +{
>> +virDomainPtr dom;
>> +const char *name = NULL;
>> +char *file = NULL;
>> +int fd = -1;
>> +virStreamPtr st = NULL;
>> +unsigned int screen = 0;
>> +unsigned int flags = 0; /* currently unused */
>> +int ret = false;
>> +bool created = true;
>> +bool generated = false;
>> +char *mime = NULL;
>> +
>> +if (!vshConnectionUsability(ctl, ctl->conn))
>> +return false;
>> +
>> +if (vshCommandOptString(cmd, "file", (const char **) &file) < 0) {
>> +vshError(ctl, "%s", _("file must not be empty"));
>> +return false;
>> +}
>> +
>> +if (vshCommandOptUInt(cmd, "screen", &screen) < 0) {
>> +vshError(ctl, "%s", _("invalid screen ID"));
>> +return false;
>> +}
>> +
>> +if (!(dom = vshCommandOptDomai
Re: [libvirt] [PATCH] Add call to sanlock_restrict() in QEMU lock driver
On Thu, Jun 02, 2011 at 11:52:57AM +0100, Daniel P. Berrange wrote:
> In between fork and exec, a connection to sanlock is acquired
> and the socket file descriptor is intionally leaked to the
> child process. sanlock watches this FD for POLL_HANGUP to
> detect when QEMU has exited. We don't want a rogus/compromised
> QEMU from issuing sanlock RPC calls on the leaked FD though,
> since that could be used to DOS other guests. By calling
> sanlock_restrict() on the socket before exec() we can lock
> it down.
>
> * configure.ac: Check for sanlock_restrict API
> * src/locking/domain_lock.c: Restrict lock acquired in
> process startup phase
> * src/locking/lock_driver.h: Add VIR_LOCK_MANAGER_ACQUIRE_RESTRICT
> * src/locking/lock_driver_sanlock.c: Add call to sanlock_restrict
> when requested by VIR_LOCK_MANAGER_ACQUIRE_RESTRICT flag
> ---
> configure.ac |2 +-
> src/locking/domain_lock.c |8 +---
> src/locking/lock_driver.h |4 +++-
> src/locking/lock_driver_sanlock.c | 15 ++-
> 4 files changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index a1bd64d..25669cf 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -972,7 +972,7 @@ if test "x$with_sanlock" != "xno"; then
> fail=1
> fi])
>if test "x$with_sanlock" != "xno" ; then
> -AC_CHECK_LIB([sanlock], [sanlock_acquire],[
> +AC_CHECK_LIB([sanlock], [sanlock_restrict],[
>SANLOCK_LIBS="$SANLOCK_LIBS -lsanlock"
>with_sanlock=yes
> ],[
> diff --git a/src/locking/domain_lock.c b/src/locking/domain_lock.c
> index 85352e2..f0a11b7 100644
> --- a/src/locking/domain_lock.c
> +++ b/src/locking/domain_lock.c
> @@ -159,10 +159,12 @@ int virDomainLockProcessStart(virLockManagerPluginPtr
> plugin,
> {
> virLockManagerPtr lock = virDomainLockManagerNew(plugin, dom, true);
> int ret;
> +int flags = VIR_LOCK_MANAGER_ACQUIRE_RESTRICT;
> +
> if (paused)
> -ret = virLockManagerAcquire(lock, NULL,
> VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY);
> -else
> -ret = virLockManagerAcquire(lock, NULL, 0);
> +flags |= VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY;
> +
> +ret = virLockManagerAcquire(lock, NULL, flags);
>
> virLockManagerFree(lock);
>
> diff --git a/src/locking/lock_driver.h b/src/locking/lock_driver.h
> index 40a55f6..2e71113 100644
> --- a/src/locking/lock_driver.h
> +++ b/src/locking/lock_driver.h
> @@ -59,7 +59,9 @@ typedef enum {
>
> typedef enum {
> /* Don't acquire the resources, just register the object PID */
> -VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY = (1 << 0)
> +VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY = (1 << 0),
> +/* Prevent further lock/unlock calls from this process */
> +VIR_LOCK_MANAGER_ACQUIRE_RESTRICT = (1 << 1),
> } virLockManagerAcquireFlags;
>
> enum {
> diff --git a/src/locking/lock_driver_sanlock.c
> b/src/locking/lock_driver_sanlock.c
> index 7e0610d..a60d7ce 100644
> --- a/src/locking/lock_driver_sanlock.c
> +++ b/src/locking/lock_driver_sanlock.c
> @@ -240,7 +240,8 @@ static int virLockManagerSanlockAcquire(virLockManagerPtr
> lock,
> int rv;
> int i;
>
> -virCheckFlags(VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY, -1);
> +virCheckFlags(VIR_LOCK_MANAGER_ACQUIRE_RESTRICT |
> + VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY, -1);
>
> if (priv->res_count == 0 &&
> priv->hasRWDisks) {
> @@ -327,6 +328,18 @@ static int
> virLockManagerSanlockAcquire(virLockManagerPtr lock,
> virSetInherit(sock, true) < 0)
> goto error;
>
> +if (flags & VIR_LOCK_MANAGER_ACQUIRE_RESTRICT) {
> +if ((rv = sanlock_restrict(sock, SANLK_RESTRICT_ALL)) < 0) {
> +if (rv <= -200)
> +virLockError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to restrict process: error %d"), rv);
> +else
> +virReportSystemError(-rv, "%s",
> + _("Failed to restrict process"));
> +goto error;
> +}
> +}
> +
> VIR_DEBUG("Acquire completed fd=%d", sock);
>
> if (res_free) {
Qualify as security bug fix, ACK, lease push :-)
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix handling of VIR_EVENT_HANDLE_ERROR in QEMU monitor
On Thu, Jun 02, 2011 at 11:52:10AM +0100, Daniel P. Berrange wrote:
> Commit 4454a9efc728b91e791b1f14c26ea23a19d57f48 introduced bad
> behaviour on the VIR_EVENT_HANDLE_ERROR condition. This condition
> is only hit when an invalid FD is used in poll() (typically due
> to a double-close bug). The QEMU monitor code was treating this
> condition as non-fatal, and thus libvirt would poll() in a fast
> loop forever burning 100% CPU. VIR_EVENT_HANDLE_ERROR must be
> handled in the same way as VIR_EVENT_HANDLE_HANGUP, killing the
> QEMU instance.
>
> * src/qemu/qemu_monitor.c: Treat VIR_EVENT_HANDLE_ERROR as EOF
> ---
> src/qemu/qemu_monitor.c |9 +
> 1 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 09a1b8e..26bb814 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -535,14 +535,14 @@ qemuMonitorIO(int watch, int fd, int events, void
> *opaque) {
> #endif
>
> if (mon->fd != fd || mon->watch != watch) {
> -if (events & VIR_EVENT_HANDLE_HANGUP)
> +if (events & (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR))
> eof = true;
> qemuReportError(VIR_ERR_INTERNAL_ERROR,
> _("event from unexpected fd %d!=%d / watch %d!=%d"),
> mon->fd, fd, mon->watch, watch);
> error = true;
> } else if (mon->lastError.code != VIR_ERR_OK) {
> -if (events & VIR_EVENT_HANDLE_HANGUP)
> +if (events & (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR))
> eof = true;
> error = true;
> } else {
> @@ -581,8 +581,9 @@ qemuMonitorIO(int watch, int fd, int events, void
> *opaque) {
> if (!error && !eof &&
> events & VIR_EVENT_HANDLE_ERROR) {
> qemuReportError(VIR_ERR_INTERNAL_ERROR,
> -_("Error while waiting for monitor"));
> -error = 1;
> +_("Invalid file descriptor while waiting for
> monitor"));
> +eof = 1;
> +events &= ~VIR_EVENT_HANDLE_ERROR;
> }
> if (!error && events) {
> qemuReportError(VIR_ERR_INTERNAL_ERROR,
Hum, that serious ! ACK
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] screenshot: Expose the new API in virsh
On Thu, Jun 02, 2011 at 01:26:22PM +0200, Michal Privoznik wrote:
> * tools/virsh.c: Add screenshot command
> * tools/virsh.pod: Document new command
> * src/libvirt.c: Fix off-be-one error
> ---
> diff to v1:
> - make filename optional and generate filename when missing
>
> diff to v2:
> - Eric's review suggestions included
>
> src/libvirt.c |2 +-
> tools/virsh.c | 187 +++---
> tools/virsh.pod |9 +++
> 3 files changed, 186 insertions(+), 12 deletions(-)
>
> diff --git a/src/libvirt.c b/src/libvirt.c
> index ee5c7cd..eaae0ec 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -2464,7 +2464,7 @@ error:
> * The screen ID is the sequential number of screen. In case of multiple
> * graphics cards, heads are enumerated before devices, e.g. having
> * two graphics cards, both with four heads, screen ID 5 addresses
> - * the first head on the second card.
> + * the second head on the second card.
> *
> * Returns a string representing the mime-type of the image format, or
> * NULL upon error. The caller must free() the returned value.
> diff --git a/tools/virsh.c b/tools/virsh.c
> index dfd5bd2..da10a0b 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -264,6 +264,9 @@ static bool vshCmdGrpHelp(vshControl *ctl, const char
> *name);
> static vshCmdOpt *vshCommandOpt(const vshCmd *cmd, const char *name);
> static int vshCommandOptInt(const vshCmd *cmd, const char *name, int *value)
> ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
> +static int vshCommandOptUInt(const vshCmd *cmd, const char *name,
> + unsigned int *value)
> +ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
> static int vshCommandOptUL(const vshCmd *cmd, const char *name,
> unsigned long *value)
> ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
> @@ -1938,6 +1941,153 @@ cmdDump(vshControl *ctl, const vshCmd *cmd)
> return ret;
> }
>
> +static const vshCmdInfo info_screenshot[] = {
> +{"help", N_("take a screenshot of a current domain console and store it "
> +"into a file")},
> +{"desc", N_("screenshot of a current domain console")},
> +{NULL, NULL}
> +};
> +
> +static const vshCmdOptDef opts_screenshot[] = {
> +{"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
> +{"file", VSH_OT_DATA, VSH_OFLAG_NONE, N_("where to store the
> screenshot")},
> +{"screen", VSH_OT_INT, VSH_OFLAG_NONE, N_("ID of a screen to take
> screenshot of")},
> +{NULL, 0, 0, NULL}
> +};
> +
> +static int vshStreamSink(virStreamPtr st ATTRIBUTE_UNUSED,
> + const char *bytes, size_t nbytes, void *opaque)
> +{
> +int *fd = opaque;
> +
> +return safewrite(*fd, bytes, nbytes);
> +}
> +
> +/**
> + * Generate string: '-[]'
> + */
> +static char *
> +vshGenFileName(vshControl *ctl, virDomainPtr dom, const char *mime)
> +{
> +char timestr[100];
> +struct timeval cur_time;
> +struct tm time_info;
> +const char *ext = NULL;
> +char *ret = NULL;
> +
> +/* We should be already connected, but doesn't
> + * hurt to check */
> +if (!vshConnectionUsability(ctl, ctl->conn))
> +return NULL;
> +
> +if (!dom) {
> +vshError(ctl, "%s", _("Invalid domain supplied"));
> +return NULL;
> +}
> +
> +if (STREQ(mime, "image/x-portable-pixmap"))
> +ext = ".ppm";
> +else if (STREQ(mime, "image/png"))
> +ext = ".png";
> +/* add mime type here */
> +
> +gettimeofday(&cur_time, NULL);
> +localtime_r(&cur_time.tv_sec, &time_info);
> +strftime(timestr, sizeof(timestr), "%Y-%m-%d-%H:%M:%S", &time_info);
> +
> +if (virAsprintf(&ret, "%s-%s%s", virDomainGetName(dom),
> +timestr, ext ? ext : "") < 0) {
> +vshError(ctl, "%s", _("Out of memory"));
> +return NULL;
> +}
> +
> +return ret;
> +}
> +
> +static bool
> +cmdScreenshot(vshControl *ctl, const vshCmd *cmd)
> +{
> +virDomainPtr dom;
> +const char *name = NULL;
> +char *file = NULL;
> +int fd = -1;
> +virStreamPtr st = NULL;
> +unsigned int screen = 0;
> +unsigned int flags = 0; /* currently unused */
> +int ret = false;
> +bool created = true;
> +bool generated = false;
> +char *mime = NULL;
> +
> +if (!vshConnectionUsability(ctl, ctl->conn))
> +return false;
> +
> +if (vshCommandOptString(cmd, "file", (const char **) &file) < 0) {
> +vshError(ctl, "%s", _("file must not be empty"));
> +return false;
> +}
> +
> +if (vshCommandOptUInt(cmd, "screen", &screen) < 0) {
> +vshError(ctl, "%s", _("invalid screen ID"));
> +return false;
> +}
> +
> +if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
> +return false;
> +
> +st = virStreamNew(ctl->conn, 0);
> +
> +mime = virDomainScreenshot(dom, st, screen, flags);
> +if (!mime) {
> +vshError
Re: [libvirt] [PATCH] build: avoid corrupting / in RHEL 5
On Thu, Jun 02, 2011 at 05:48:39AM -0600, Eric Blake wrote: > On 06/02/2011 05:26 AM, Daniel P. Berrange wrote: > > > > I've seen a strange build error from autobuild.sh which I'm thinking > > may be related to this patch > > > > Generating drivers.html.tmp > > Stubbing todo.html.in > > Generating internals/locking.html.tmp > > Generating internals/command.html.tmp > > There's no locking.html.in yet (except in your patch series), so this > points to a bug in at least the VPATH rules for your conversion from > locking.html.in to locking.html. > > You need to generate locking.html in $(srcdir), since it will be > distributed pre-built as part of the tarball. There's no special makefile rules for locking.html - all the HTML stuff is wildcarded. The same error hits command.html here too. > > > GEN../../docs/libvirt-api.xml > > /bin/sh: line 3: internals/locking.html.tmp: No such file or directory > > /bin/sh: line 3: internals/command.html.tmp: No such file or directory > > Validating architecture.html > > Validating contact.html > > rm: cannot remove `internals/command.html.tmp': No such file or directory > > make[2]: *** [internals/command.html.tmp] Error 1 > > make[2]: *** Waiting for unfinished jobs > > Sometimes, it helps to use make -j1 to ensure that interleaved output > from parallel rules isn't causing further confusion, although I don't > know if it's easy to do that as an autobuild.sh parameter or if it > requires an actual script tweak. Yep, you can already just run MAKEFLAGS="-j1" ./autobuild.sh Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] libxl driver additions
On Wed, Jun 01, 2011 at 12:55:32PM +0200, Markus Groß wrote: > I know it is very late for 0.9.2 but perhabs we could > squeeze these patches in (then the version numbers > of the functions in patch 2 have to be changed to 0.9.2). > > Markus Groß (2): > Get maximum memory of running domain in libxl driver > Add managedSave support to libxl driver > > src/libxl/libxl_driver.c | 356 > -- > 1 files changed, 278 insertions(+), 78 deletions(-) okay, the first one was a no brainer, but the second one is more consistent and modifies (a bit) existing entry points. Still I looked and pushed both as it doesn't look risky, I will try to give it a bit more checking during the week-end though (I'm travelling now !) thanks, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: avoid corrupting / in RHEL 5
On 06/02/2011 05:26 AM, Daniel P. Berrange wrote: > > I've seen a strange build error from autobuild.sh which I'm thinking > may be related to this patch > > Generating drivers.html.tmp > Stubbing todo.html.in > Generating internals/locking.html.tmp > Generating internals/command.html.tmp There's no locking.html.in yet (except in your patch series), so this points to a bug in at least the VPATH rules for your conversion from locking.html.in to locking.html. You need to generate locking.html in $(srcdir), since it will be distributed pre-built as part of the tarball. > GEN../../docs/libvirt-api.xml > /bin/sh: line 3: internals/locking.html.tmp: No such file or directory > /bin/sh: line 3: internals/command.html.tmp: No such file or directory > Validating architecture.html > Validating contact.html > rm: cannot remove `internals/command.html.tmp': No such file or directory > make[2]: *** [internals/command.html.tmp] Error 1 > make[2]: *** Waiting for unfinished jobs Sometimes, it helps to use make -j1 to ensure that interleaved output from parallel rules isn't causing further confusion, although I don't know if it's easy to do that as an autobuild.sh parameter or if it requires an actual script tweak. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: avoid corrupting / in RHEL 5
On Wed, Jun 01, 2011 at 11:52:00AM -0600, Eric Blake wrote:
> I noticed this while building from libvirt.git on RHEL 5.6:
>
> Generating internals/command.html.tmp
> mkdir: cannot create directory `/internals': Permission denied
>
> If I had been building as root instead, this pollutes /.
>
> Older autoconf lacks $(builddir), but it is rigorously equal to '.'
> in newer autoconf, so we could use '$(MKDIR_P) internals' instead.
>
> However, since internals/command.html is part of the tarball, we
> _already_ build it in $(srcdir), not $(builddir) during VPATH
> builds, so the mkdir is wasted effort!
>
> * docs/Makefile.am (internals/%.html.tmp): Drop unused mkdir.
> ---
> docs/Makefile.am |1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/docs/Makefile.am b/docs/Makefile.am
> index a8024b3..a98ced0 100644
> --- a/docs/Makefile.am
> +++ b/docs/Makefile.am
> @@ -127,7 +127,6 @@ hvsupport.html.in: $(srcdir)/hvsupport.pl
> $(srcdir)/../src/libvirt_public.syms \
> internals/%.html.tmp: internals/%.html.in subsite.xsl page.xsl
> sitemap.html.in
> @if [ -x $(XSLTPROC) ] ; then \
> echo "Generating $@"; \
> - $(MKDIR_P) "$(builddir)/internals"; \
> name=`echo $@ | sed -e 's/.tmp//'`; \
> $(XSLTPROC) --stringparam pagename $$name --nonet --html \
> $(top_srcdir)/docs/subsite.xsl $< > $@ \
I've seen a strange build error from autobuild.sh which I'm thinking
may be related to this patch
...
Generating drvvmware.html.tmp
Generating firewall.html.tmp
Generating bindings.html.tmp
if [ -f todo.cfg ]; then \
echo "Generating todo.html.in"; \
perl ../../docs/../../docs/todo.pl > todo.html.in \
|| { rm todo.html.in && exit 1; }; \
else \
echo "Stubbing todo.html.in"; \
echo "Todo list" > todo.html.in ; \
fi
Generating drivers.html.tmp
Stubbing todo.html.in
Generating internals/locking.html.tmp
Generating internals/command.html.tmp
GEN../../docs/libvirt-api.xml
/bin/sh: line 3: internals/locking.html.tmp: No such file or directory
/bin/sh: line 3: internals/command.html.tmp: No such file or directory
Validating architecture.html
Validating contact.html
rm: cannot remove `internals/command.html.tmp': No such file or directory
make[2]: *** [internals/command.html.tmp] Error 1
make[2]: *** Waiting for unfinished jobs
rm: cannot remove `internals/locking.html.tmp': No such file or directory
make[2]: *** [internals/locking.html.tmp] Error 1
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3] screenshot: Expose the new API in virsh
* tools/virsh.c: Add screenshot command
* tools/virsh.pod: Document new command
* src/libvirt.c: Fix off-be-one error
---
diff to v1:
- make filename optional and generate filename when missing
diff to v2:
- Eric's review suggestions included
src/libvirt.c |2 +-
tools/virsh.c | 187 +++---
tools/virsh.pod |9 +++
3 files changed, 186 insertions(+), 12 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index ee5c7cd..eaae0ec 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -2464,7 +2464,7 @@ error:
* The screen ID is the sequential number of screen. In case of multiple
* graphics cards, heads are enumerated before devices, e.g. having
* two graphics cards, both with four heads, screen ID 5 addresses
- * the first head on the second card.
+ * the second head on the second card.
*
* Returns a string representing the mime-type of the image format, or
* NULL upon error. The caller must free() the returned value.
diff --git a/tools/virsh.c b/tools/virsh.c
index dfd5bd2..da10a0b 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -264,6 +264,9 @@ static bool vshCmdGrpHelp(vshControl *ctl, const char
*name);
static vshCmdOpt *vshCommandOpt(const vshCmd *cmd, const char *name);
static int vshCommandOptInt(const vshCmd *cmd, const char *name, int *value)
ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
+static int vshCommandOptUInt(const vshCmd *cmd, const char *name,
+ unsigned int *value)
+ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
static int vshCommandOptUL(const vshCmd *cmd, const char *name,
unsigned long *value)
ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
@@ -1938,6 +1941,153 @@ cmdDump(vshControl *ctl, const vshCmd *cmd)
return ret;
}
+static const vshCmdInfo info_screenshot[] = {
+{"help", N_("take a screenshot of a current domain console and store it "
+"into a file")},
+{"desc", N_("screenshot of a current domain console")},
+{NULL, NULL}
+};
+
+static const vshCmdOptDef opts_screenshot[] = {
+{"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
+{"file", VSH_OT_DATA, VSH_OFLAG_NONE, N_("where to store the screenshot")},
+{"screen", VSH_OT_INT, VSH_OFLAG_NONE, N_("ID of a screen to take
screenshot of")},
+{NULL, 0, 0, NULL}
+};
+
+static int vshStreamSink(virStreamPtr st ATTRIBUTE_UNUSED,
+ const char *bytes, size_t nbytes, void *opaque)
+{
+int *fd = opaque;
+
+return safewrite(*fd, bytes, nbytes);
+}
+
+/**
+ * Generate string: '-[]'
+ */
+static char *
+vshGenFileName(vshControl *ctl, virDomainPtr dom, const char *mime)
+{
+char timestr[100];
+struct timeval cur_time;
+struct tm time_info;
+const char *ext = NULL;
+char *ret = NULL;
+
+/* We should be already connected, but doesn't
+ * hurt to check */
+if (!vshConnectionUsability(ctl, ctl->conn))
+return NULL;
+
+if (!dom) {
+vshError(ctl, "%s", _("Invalid domain supplied"));
+return NULL;
+}
+
+if (STREQ(mime, "image/x-portable-pixmap"))
+ext = ".ppm";
+else if (STREQ(mime, "image/png"))
+ext = ".png";
+/* add mime type here */
+
+gettimeofday(&cur_time, NULL);
+localtime_r(&cur_time.tv_sec, &time_info);
+strftime(timestr, sizeof(timestr), "%Y-%m-%d-%H:%M:%S", &time_info);
+
+if (virAsprintf(&ret, "%s-%s%s", virDomainGetName(dom),
+timestr, ext ? ext : "") < 0) {
+vshError(ctl, "%s", _("Out of memory"));
+return NULL;
+}
+
+return ret;
+}
+
+static bool
+cmdScreenshot(vshControl *ctl, const vshCmd *cmd)
+{
+virDomainPtr dom;
+const char *name = NULL;
+char *file = NULL;
+int fd = -1;
+virStreamPtr st = NULL;
+unsigned int screen = 0;
+unsigned int flags = 0; /* currently unused */
+int ret = false;
+bool created = true;
+bool generated = false;
+char *mime = NULL;
+
+if (!vshConnectionUsability(ctl, ctl->conn))
+return false;
+
+if (vshCommandOptString(cmd, "file", (const char **) &file) < 0) {
+vshError(ctl, "%s", _("file must not be empty"));
+return false;
+}
+
+if (vshCommandOptUInt(cmd, "screen", &screen) < 0) {
+vshError(ctl, "%s", _("invalid screen ID"));
+return false;
+}
+
+if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
+return false;
+
+st = virStreamNew(ctl->conn, 0);
+
+mime = virDomainScreenshot(dom, st, screen, flags);
+if (!mime) {
+vshError(ctl, _("could not take a screenshot of %s"), name);
+goto cleanup;
+}
+
+if (!file) {
+if (!(file=vshGenFileName(ctl, dom, mime)))
+return false;
+generated = true;
+}
+
+if ((fd = open(file, O_WRONLY|O_CREAT|O_EXCL, 0666)) < 0) {
+created = false;
+if (errno != EEXIST ||
+(fd = op
[libvirt] [PATCH] Add call to sanlock_restrict() in QEMU lock driver
In between fork and exec, a connection to sanlock is acquired
and the socket file descriptor is intionally leaked to the
child process. sanlock watches this FD for POLL_HANGUP to
detect when QEMU has exited. We don't want a rogus/compromised
QEMU from issuing sanlock RPC calls on the leaked FD though,
since that could be used to DOS other guests. By calling
sanlock_restrict() on the socket before exec() we can lock
it down.
* configure.ac: Check for sanlock_restrict API
* src/locking/domain_lock.c: Restrict lock acquired in
process startup phase
* src/locking/lock_driver.h: Add VIR_LOCK_MANAGER_ACQUIRE_RESTRICT
* src/locking/lock_driver_sanlock.c: Add call to sanlock_restrict
when requested by VIR_LOCK_MANAGER_ACQUIRE_RESTRICT flag
---
configure.ac |2 +-
src/locking/domain_lock.c |8 +---
src/locking/lock_driver.h |4 +++-
src/locking/lock_driver_sanlock.c | 15 ++-
4 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/configure.ac b/configure.ac
index a1bd64d..25669cf 100644
--- a/configure.ac
+++ b/configure.ac
@@ -972,7 +972,7 @@ if test "x$with_sanlock" != "xno"; then
fail=1
fi])
if test "x$with_sanlock" != "xno" ; then
-AC_CHECK_LIB([sanlock], [sanlock_acquire],[
+AC_CHECK_LIB([sanlock], [sanlock_restrict],[
SANLOCK_LIBS="$SANLOCK_LIBS -lsanlock"
with_sanlock=yes
],[
diff --git a/src/locking/domain_lock.c b/src/locking/domain_lock.c
index 85352e2..f0a11b7 100644
--- a/src/locking/domain_lock.c
+++ b/src/locking/domain_lock.c
@@ -159,10 +159,12 @@ int virDomainLockProcessStart(virLockManagerPluginPtr
plugin,
{
virLockManagerPtr lock = virDomainLockManagerNew(plugin, dom, true);
int ret;
+int flags = VIR_LOCK_MANAGER_ACQUIRE_RESTRICT;
+
if (paused)
-ret = virLockManagerAcquire(lock, NULL,
VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY);
-else
-ret = virLockManagerAcquire(lock, NULL, 0);
+flags |= VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY;
+
+ret = virLockManagerAcquire(lock, NULL, flags);
virLockManagerFree(lock);
diff --git a/src/locking/lock_driver.h b/src/locking/lock_driver.h
index 40a55f6..2e71113 100644
--- a/src/locking/lock_driver.h
+++ b/src/locking/lock_driver.h
@@ -59,7 +59,9 @@ typedef enum {
typedef enum {
/* Don't acquire the resources, just register the object PID */
-VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY = (1 << 0)
+VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY = (1 << 0),
+/* Prevent further lock/unlock calls from this process */
+VIR_LOCK_MANAGER_ACQUIRE_RESTRICT = (1 << 1),
} virLockManagerAcquireFlags;
enum {
diff --git a/src/locking/lock_driver_sanlock.c
b/src/locking/lock_driver_sanlock.c
index 7e0610d..a60d7ce 100644
--- a/src/locking/lock_driver_sanlock.c
+++ b/src/locking/lock_driver_sanlock.c
@@ -240,7 +240,8 @@ static int virLockManagerSanlockAcquire(virLockManagerPtr
lock,
int rv;
int i;
-virCheckFlags(VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY, -1);
+virCheckFlags(VIR_LOCK_MANAGER_ACQUIRE_RESTRICT |
+ VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY, -1);
if (priv->res_count == 0 &&
priv->hasRWDisks) {
@@ -327,6 +328,18 @@ static int virLockManagerSanlockAcquire(virLockManagerPtr
lock,
virSetInherit(sock, true) < 0)
goto error;
+if (flags & VIR_LOCK_MANAGER_ACQUIRE_RESTRICT) {
+if ((rv = sanlock_restrict(sock, SANLK_RESTRICT_ALL)) < 0) {
+if (rv <= -200)
+virLockError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to restrict process: error %d"), rv);
+else
+virReportSystemError(-rv, "%s",
+ _("Failed to restrict process"));
+goto error;
+}
+}
+
VIR_DEBUG("Acquire completed fd=%d", sock);
if (res_free) {
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix handling of VIR_EVENT_HANDLE_ERROR in QEMU monitor
Commit 4454a9efc728b91e791b1f14c26ea23a19d57f48 introduced bad
behaviour on the VIR_EVENT_HANDLE_ERROR condition. This condition
is only hit when an invalid FD is used in poll() (typically due
to a double-close bug). The QEMU monitor code was treating this
condition as non-fatal, and thus libvirt would poll() in a fast
loop forever burning 100% CPU. VIR_EVENT_HANDLE_ERROR must be
handled in the same way as VIR_EVENT_HANDLE_HANGUP, killing the
QEMU instance.
* src/qemu/qemu_monitor.c: Treat VIR_EVENT_HANDLE_ERROR as EOF
---
src/qemu/qemu_monitor.c |9 +
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 09a1b8e..26bb814 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -535,14 +535,14 @@ qemuMonitorIO(int watch, int fd, int events, void
*opaque) {
#endif
if (mon->fd != fd || mon->watch != watch) {
-if (events & VIR_EVENT_HANDLE_HANGUP)
+if (events & (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR))
eof = true;
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_("event from unexpected fd %d!=%d / watch %d!=%d"),
mon->fd, fd, mon->watch, watch);
error = true;
} else if (mon->lastError.code != VIR_ERR_OK) {
-if (events & VIR_EVENT_HANDLE_HANGUP)
+if (events & (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR))
eof = true;
error = true;
} else {
@@ -581,8 +581,9 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) {
if (!error && !eof &&
events & VIR_EVENT_HANDLE_ERROR) {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
-_("Error while waiting for monitor"));
-error = 1;
+_("Invalid file descriptor while waiting for
monitor"));
+eof = 1;
+events &= ~VIR_EVENT_HANDLE_ERROR;
}
if (!error && events) {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
--
1.7.4.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix netdev detection on RHEL6x versions of qemu
On Thu, Jun 02, 2011 at 10:04:44AM +0100, Neil Wilson wrote: > On Wed, 2011-06-01 at 13:38 -0600, Eric Blake wrote: > > > Meanwhile, as to your question about the libvirt testsuit including RHEL > > qemu -help output: that merely proves that libvirt is properly parsing > > the available -help output, and not that libvirt is inferring any > > special properties of RHEL qemu that were not directly advertised in -help. > > > > I appreciate your input but I feel that it suffers from the same problem > you complain of. > > Libvirt is not properly parsing the help message as the decisions it > makes means that the attach-device derived functions do not work with > the qemu supplied on a major platform. > > The actual fault is with 12.x standard qemu advertising netdev when it > doesn't have it. Libvirt is using 'special properties not directly > advertised in -help' to infer that 12.x netdev is broken. 12.x does have netdev, but we don't want to use it because it does not support hotplug, only with initial startup. Hence it is better to continue using the old network syntax to avoid loosing functionality Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix QEMU XML-2-ARGV graphics-spice-timeout test
On Wed, Jun 01, 2011 at 02:26:54PM -0600, Eric Blake wrote: > On 06/01/2011 10:13 AM, Matthias Bolte wrote: > > The test used an emulator that is not supported in testutilsqemu.c. > > Swicth from qemu-kvm to kvm to fix this. > > --- > > > > This patch addresses the problem discussed here: > > https://www.redhat.com/archives/libvir-list/2011-May/msg01914.html > > Shoot, now I'm getting failures on Fedora 14: > > 55) QEMU XML-2-ARGV graphics-spice-timeout > ... libvir: error : internal error Child process (LC_ALL=C PATH=/bin > HOME=/home/test USER=test LOGNAME=test /usr/bin/kvm -cpu ?) status > unexpected: exit status 1 > FAILED That's because the graphics-spice-timeout XML has a full model and flags present. This is irrelevant for this test, so just remove it and everything will be fine. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: Document nodeinfo output
--- tools/virsh.pod |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index 9251db6..1f41652 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -235,7 +235,9 @@ Print the XML representation of the hypervisor sysinfo, if available. =item B Returns basic information about the node, like number and type of CPU, -and size of the physical memory. +and size of the physical memory. The output corresponds to virNodeInfo +structure. Specifically, the "CPU socket(s)" field means number of CPU +sockets per NUMA cell. =item B -- 1.7.5.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] docs: Add doc for video element
For backwards compatability, if no is set but there is a tag, then we add a default according to the guest type. Add docs to tell the user about this to not make them confused. Especially if they remove the video (such as via "virsh edit"), it will be surprised for them to see the video element is still in domain XML. --- docs/formatdomain.html.in |7 ++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f8baffd..455f4dd 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1868,7 +1868,12 @@ qemu-kvm -net nic,model=? /dev/null video The video element is the a container for describing -video devices. +video devices. NB, for backwards compatability, if no video +is set but there is a graphics in domain xml, then libvirt +will add a default video according to the guest type, e.g. +For a guest of type "kvm", the default video for it is: +type with value "cirrus", vram with value "9216", +and heads with value "1". model -- 1.7.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Use VIR_USE_CPU instead of new wheel
* src/libxl/libxl_driver.c
* src/qemu/qemu_process.c
---
src/libxl/libxl_driver.c |7 ++-
src/qemu/qemu_process.c | 13 ++---
2 files changed, 4 insertions(+), 16 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 3491f40..aaab044 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -450,7 +450,6 @@ libxlDomainSetVcpuAffinites(libxlDriverPrivatePtr driver,
virDomainObjPtr vm)
uint8_t *cpumap = NULL;
virNodeInfo nodeinfo;
size_t cpumaplen;
-unsigned int pos;
int vcpu, i;
int ret = -1;
@@ -471,10 +470,8 @@ libxlDomainSetVcpuAffinites(libxlDriverPrivatePtr driver,
virDomainObjPtr vm)
cpumask = (uint8_t*) def->cputune.vcpupin[vcpu]->cpumask;
for (i = 0; i < VIR_DOMAIN_CPUMASK_LEN; ++i) {
-if (cpumask[i]) {
-pos = i / 8;
-cpumap[pos] |= 1 << (i % 8);
-}
+if (cpumask[i])
+VIR_USE_CPU(cpumap, i);
}
map.size = cpumaplen;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 811fa28..fb18fc8 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1212,18 +1212,9 @@ qemuProcessSetVcpuAffinites(virConnectPtr conn,
cpumask = (unsigned char *)def->cputune.vcpupin[vcpu]->cpumask;
vcpupid = priv->vcpupids[vcpu];
-/* Convert cpumask to bitmap here. */
-for (i = 0; i < VIR_DOMAIN_CPUMASK_LEN; i++) {
-int cur = 0;
-int mod = 0;
-
-if (i) {
-cur = i / 8;
-mod = i % 8;
-}
-
+for (i = 0 ; i < VIR_DOMAIN_CPUMASK_LEN ; i++) {
if (cpumask[i])
-cpumap[cur] |= 1 << mod;
+VIR_USE_CPU(cpumap, i);
}
if (virProcessInfoSetAffinity(vcpupid,
--
1.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix netdev detection on RHEL6x versions of qemu
On Wed, 2011-06-01 at 13:38 -0600, Eric Blake wrote: > Meanwhile, as to your question about the libvirt testsuit including RHEL > qemu -help output: that merely proves that libvirt is properly parsing > the available -help output, and not that libvirt is inferring any > special properties of RHEL qemu that were not directly advertised in -help. > I appreciate your input but I feel that it suffers from the same problem you complain of. Libvirt is not properly parsing the help message as the decisions it makes means that the attach-device derived functions do not work with the qemu supplied on a major platform. The actual fault is with 12.x standard qemu advertising netdev when it doesn't have it. Libvirt is using 'special properties not directly advertised in -help' to infer that 12.x netdev is broken. So really you're special casing the standard qemu, and actually you'd probably be better removing that special case altogether and letting the standard qemu fail on attach (which frankly less people will be using), and feeding the problem upstream to the qemu developer to fix the 12.x branch help message. I'll leave it with you guys at that. The patch is on the list for anybody suffering the same issues I am. NeilW -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
