May 2014 - Devel - libvirt List Archives

[libvirt] [Question] (python API) setMemoryParameters doesn't support flags CONFIG | LIVE
by Wangrui (K) 13 May '14

13 May '14

Hi, all when I use (python API) setMemoryParameters with flags= VIR_DOMAIN_AFFECT_CONFIG|VIR_DOMAIN_AFFECT_LIVE , an error occurs as follow: (libvirt 1.2.4) libvirt: error : flags 'affect live' and 'affect config' in virDomainGetMemoryParameters are mutually exclusive I check the function libvirt_virDomainSetMemoryParameters in libvirt-override.c and find the code below: Line 1204-1206: LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainGetMemoryParameters(domain, NULL, &nparams, flags); LIBVIRT_END_ALLOW_THREADS; Line 1220-1222: LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainGetMemoryParameters(domain, params, &nparams, flags); LIBVIRT_END_ALLOW_THREADS; Function virDomainGetMemoryParameters doesn't support flags VIR_DOMAIN_AFFECT_CONFIG|VIR_DOMAIN_AFFECT_LIVE. So the error above occurs. The first call of virDomainGetMemoryParameters is to "cause @nparams on output to contain the number of parameters supported by the hypervisor". And what's the role of the second call? Maybe, so do other functions such as *libvirt_virDomainSetSchedulerParameters, *libvirt_virDomainSetBlkioParameters In commit 56cec18d761a1f99862c43811de60380c65881e6. IMHO, an example flag (such as VIR_DOMAIN_AFFECT_CONFIG) can be used in virDomainGetMemoryParameters just to get something from hypervisor rather than @flags which is a parameter of setMemoryParameters .

1 0

[libvirt] [PATCH] Improve the error info when fail to start guest due to lack of cgroup controller
by Shanzhi Yu 13 May '14

13 May '14

When cgroup controller cpu, cpuset and cpuacct are all unmounted, fail to start guest. The error info is not accurate. It will report "At least one cgroup controller is required" even though there is cgroup controller memory or blkio. Signed-off-by: Shanzhi Yu <shyu(a)redhat.com> --- src/util/vircgroup.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index fce380a..c609eb3 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -586,6 +586,11 @@ virCgroupDetect(virCgroupPtr group, if (!group->controllers[i].mountPoint) { VIR_DEBUG("Requested controller '%s' not mounted, ignoring", virCgroupControllerTypeToString(i)); + if ( i==2 ) { + virReportSystemError(ENXIO, "%s", + _("At least one cgroup controller cpu or cpuacct or cpuset is required")); + return -1; + } controllers &= ~(1 << i); } } else { -- 1.8.3.1

1 0

[libvirt] [PATCH] sanlock: avoid leak in acquire()
by Martin Kletzander 13 May '14

13 May '14

Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- src/locking/lock_driver_sanlock.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c index b3d31bf..e4b4e8e 100644 --- a/src/locking/lock_driver_sanlock.c +++ b/src/locking/lock_driver_sanlock.c @@ -907,9 +907,6 @@ static int virLockManagerSanlockAcquire(virLockManagerPtr lock, return -1; } - if (VIR_ALLOC(opt) < 0) - return -1; - /* We only initialize 'sock' if we are in the real * child process and we need it to be inherited * @@ -944,6 +941,9 @@ static int virLockManagerSanlockAcquire(virLockManagerPtr lock, return 0; } + if (VIR_ALLOC(opt) < 0) + return -1; + /* sanlock doesn't use owner_name for anything, so it's safe to take just * the first SANLK_NAME_LEN - 1 characters from vm_name */ ignore_value(virStrncpy(opt->owner_name, priv->vm_name, -- 1.9.3

2 2

[libvirt] [PATCH 1/2] sanlock: code movement in virLockManagerSanlockAcquire
by Martin Kletzander 13 May '14

13 May '14

Just move some code around for future patches to ease the review. With this patch there is no need for drastic cleanup path later. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- src/locking/lock_driver_sanlock.c | 50 +++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c index 7e4258d..01441a0 100644 --- a/src/locking/lock_driver_sanlock.c +++ b/src/locking/lock_driver_sanlock.c @@ -896,31 +896,6 @@ static int virLockManagerSanlockAcquire(virLockManagerPtr lock, if (VIR_ALLOC(opt) < 0) return -1; - /* sanlock doesn't use owner_name for anything, so it's safe to take just - * the first SANLK_NAME_LEN - 1 characters from vm_name */ - ignore_value(virStrncpy(opt->owner_name, priv->vm_name, - SANLK_NAME_LEN - 1, SANLK_NAME_LEN)); - - if (state && STRNEQ(state, "")) { - if ((rv = sanlock_state_to_args((char *)state, - &res_count, - &res_args)) < 0) { - if (rv <= -200) - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to parse lock state %s: error %d"), - state, rv); - else - virReportSystemError(-rv, - _("Unable to parse lock state %s"), - state); - goto error; - } - res_free = true; - } else { - res_args = priv->res_args; - res_count = priv->res_count; - } - /* We only initialize 'sock' if we are in the real * child process and we need it to be inherited * @@ -949,6 +924,31 @@ static int virLockManagerSanlockAcquire(virLockManagerPtr lock, } } + /* sanlock doesn't use owner_name for anything, so it's safe to take just + * the first SANLK_NAME_LEN - 1 characters from vm_name */ + ignore_value(virStrncpy(opt->owner_name, priv->vm_name, + SANLK_NAME_LEN - 1, SANLK_NAME_LEN)); + + if (state && STRNEQ(state, "")) { + if ((rv = sanlock_state_to_args((char *)state, + &res_count, + &res_args)) < 0) { + if (rv <= -200) + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse lock state %s: error %d"), + state, rv); + else + virReportSystemError(-rv, + _("Unable to parse lock state %s"), + state); + goto error; + } + res_free = true; + } else { + res_args = priv->res_args; + res_count = priv->res_count; + } + if (!(flags & VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY)) { VIR_DEBUG("Acquiring object %u", priv->res_count); if ((rv = sanlock_acquire(sock, priv->vm_pid, 0, -- 1.9.2

3 5

[libvirt] [PATCH] apibuild: Disallow 'returns' return decription
by Michal Privoznik 13 May '14

13 May '14

Our documentation generator is a bit messy, to say the least. For instance, the description to return values of a function is searched within C comment. Currently, all lines that start with 'returns' or 'Returns' are viewed as return value description. However, there are some valid uses where the 'returns' word is in the middle of a sentence describing function behavior nor the return value. For instance: virDomainDetachDeviceFlags, virConnectNetworkEventRegisterAny and virDomainGetDiskErrors. This leads to HTML documentation to be not correctly generated. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- docs/apibuild.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/apibuild.py b/docs/apibuild.py index 71494d5..5250c5a 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -926,7 +926,7 @@ class CParser: if i < len(l) and l[i] == ' ': i = i + 1 l = l[i:] - if len(l) >= 6 and l[0:7] == "returns" or l[0:7] == "Returns": + if len(l) >= 6 and l[0:7] == "Returns": try: l = string.split(l, ' ', 1)[1] except: -- 1.9.0

3 2

[libvirt] [PATCH] virsh: domain: Fix output of the VNC display number for domdisplay
by Peter Krempa 13 May '14

13 May '14

Commit 9976c4b9a665f10ab0d2071954efb7f432d194eb broke the output for VNC displays as the port number is converted to VNC display number by subtracting 5900. This yields port 0 for the first display and thus the output would be skipped. Before: $ virsh domdisplay VM vnc://localhost After: $ tools/virsh domdisplay VM vnc://localhost:0 --- tools/virsh-domain.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 54d8867..d8183ee 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9392,12 +9392,6 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) passwd = virXPathString(xpath, ctxt); VIR_FREE(xpath); - if (STREQ(scheme[iter], "vnc")) { - /* VNC protocol handlers take their port number as - * 'port' - 5900 */ - port -= 5900; - } - /* Build up the full URI, starting with the scheme */ virBufferAsprintf(&buf, "%s://", scheme[iter]); @@ -9416,8 +9410,15 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) virBufferAsprintf(&buf, "%s", listen_addr); /* Add the port */ - if (port) + if (port) { + if (STREQ(scheme[iter], "vnc")) { + /* VNC protocol handlers take their port number as + * 'port' - 5900 */ + port -= 5900; + } + virBufferAsprintf(&buf, ":%d", port); + } /* TLS Port */ if (tls_port) { -- 1.9.2

2 2

[libvirt] [PATCH v7 0/6] Expose FSFreeze/FSThaw within the guest as API
by Tomoki Sekiyama 13 May '14

13 May '14

Hello, This is patchset v7 to add FSFreeze/FSThaw API for custom disk snapshotting. Changes since v6: * return VIR_ERR_ARGUMENT_UNSUPPORTED when 'mountpoints' parameter is given even though the driver doesn't support it, rather than ignore it. * include some missing argument in debug line in libvirt.c * change option format of virsh domfsfreeze/domfsthaw from '--mountpoints <a comma separated list of mountpoints>' to '--mountpoint <mountpoint> --mountpoint <mountpoint> ...' (v6: http://www.redhat.com/archives/libvir-list/2014-April/msg01124.html ) === Description === Currently FSFreeze and FSThaw are supported by qemu guest agent and they are used internally in snapshot-create command with --quiesce option. However, when users want to utilize the native snapshot feature of storage devices (such as LVM over iSCSI, enterprise storage appliances, etc.), they need to issue fsfreeze command separately from libvirt-driven snapshots. (OpenStack cinder provides these storages' snapshot feature, but it cannot quiesce the guest filesystems automatically for now.) Although virDomainQemuGuestAgent() API could be used for this purpose, it is only for debugging and is not supported officially. This patchset adds virDomainFSFreeze()/virDomainFSThaw() APIs and virsh domfsfreeze/domfsthaw commands to enable the users to freeze and thaw domain's filesystems cleanly. The APIs take mountpoints parameters, which is a list of mountpoint paths of filesystems to be frozen/thawed. If the option is not provided, every mounted filesystem is frozen/thawed. The APIs have flags option currently unsupported for future extension. --- Tomoki Sekiyama (6): Introduce virDomainFSFreeze() and virDomainFSThaw() public API remote: Implement virDomainFSFreeze and virDomainFSThaw qemu: track quiesced status in qemuDomainSnapshotFSFreeze qemu: Implement virDomainFSFreeze and virDomainFSThaw virsh: Expose new virDomainFSFreeze and virDomainFSThaw API qemu: Support mountpoints option of guest-fsfreeze-freeze include/libvirt/libvirt.h.in | 10 +++ src/access/viraccessperm.c | 2 - src/access/viraccessperm.h | 6 ++ src/driver.h | 14 ++++ src/libvirt.c | 93 ++++++++++++++++++++++++ src/libvirt_public.syms | 6 ++ src/qemu/qemu_agent.c | 47 +++++++++++- src/qemu/qemu_agent.h | 3 + src/qemu/qemu_domain.c | 5 + src/qemu/qemu_domain.h | 2 + src/qemu/qemu_driver.c | 166 ++++++++++++++++++++++++++++++++++++++---- src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 38 +++++++++- src/remote_protocol-structs | 18 +++++ src/rpc/gendispatch.pl | 2 + tests/qemuagenttest.c | 8 +- tools/virsh-domain.c | 126 ++++++++++++++++++++++++++++++++ tools/virsh.pod | 23 ++++++ 18 files changed, 546 insertions(+), 25 deletions(-)

3 18

[libvirt] [PATCH] maint: fix typos related to 'frozen'
by Eric Blake 13 May '14

13 May '14

"Freezed" is not an English word. * src/lxc/lxc_driver.c (lxcFreezeContainer): Fix typo. * src/qemu/qemu_driver.c (qemuDomainSnapshotFSFreeze): Likewise. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- Pushing under the trivial rule. src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_driver.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index dba2182..1086289 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3231,7 +3231,7 @@ static int lxcFreezeContainer(virDomainObjPtr vm) /* * Returning EBUSY explicitly indicates that the group is - * being freezed but incomplete and other errors are true + * being frozen but incomplete, and other errors are true * errors. */ if (r < 0 && r != -EBUSY) { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4ff8a2d..e587183 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12091,7 +12091,7 @@ qemuDomainSnapshotFSFreeze(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; virQEMUDriverConfigPtr cfg; - int freezed; + int frozen; if (priv->quiesced) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", @@ -12113,9 +12113,9 @@ qemuDomainSnapshotFSFreeze(virQEMUDriverPtr driver, virObjectUnref(cfg); qemuDomainObjEnterAgent(vm); - freezed = qemuAgentFSFreeze(priv->agent); + frozen = qemuAgentFSFreeze(priv->agent); qemuDomainObjExitAgent(vm); - return freezed < 0 ? -2 : freezed; + return frozen < 0 ? -2 : frozen; } /* Return -1 on error, otherwise number of thawed filesystems. */ -- 1.9.0

1 0

Re: [libvirt] [PATCH 1/2] libxl: support syntax <interface type="hostdev">
by Jim Fehlig 12 May '14

12 May '14

Chun Yan Liu wrote: > >>>> On 5/10/2014 at 06:18 AM, in message <536D541D.5040107(a)suse.com>, Jim Fehlig >>>> > <jfehlig(a)suse.com> wrote: > >> Chunyan Liu wrote: >> >>> Signed-off-by: Chunyan Liu <cyliu(a)suse.com> >>> >>> >> >> A while back when testing Chunyan's "common hostdev library" series, I >> mentioned that <interface type='hostdev'> was not working with the libxl >> driver. Chunyan later privately sent a "v1" of this patch for testing >> in my setup. In addition to testing, I provided some private comments. >> I see those have been incorporated in this patch and functionally it >> looks good, but I do have one additional question about the commit >> message... >> >> >>> --- >>> src/libxl/libxl_conf.c | 16 +++++++++++----- >>> 1 file changed, 11 insertions(+), 5 deletions(-) >>> >>> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c >>> index 298c8a1..b7fed7f 100644 >>> --- a/src/libxl/libxl_conf.c >>> +++ b/src/libxl/libxl_conf.c >>> @@ -921,25 +921,31 @@ static int >>> libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config) >>> { >>> virDomainNetDefPtr *l_nics = def->nets; >>> - int nnics = def->nnets; >>> + size_t nnics = def->nnets; >>> libxl_device_nic *x_nics; >>> - size_t i; >>> + size_t i, nvnics = 0; >>> >>> if (VIR_ALLOC_N(x_nics, nnics) < 0) >>> return -1; >>> >>> for (i = 0; i < nnics; i++) { >>> - if (libxlMakeNic(def, l_nics[i], &x_nics[i])) >>> + if (l_nics[i]->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) >>> + continue; >>> >>> >> >> After looking at this again, it seems we are really *fixing* <interface >> type='hostdev'>. The driver already supports creating the hostdev >> device, but without this patch a libxl_device_nic is created too. Is >> that a fair statement? If so, the commit message should be changed to >> reflect this. Thanks! >> > > A NET_TYPE_HOSTDEV device is really a hostdev device, the driver will create > a hostdev device for it, so no need to create a libxl_device_nic again. Before > this patch, it tried to call libxlMakeNic to create a libxl_device_nic for it but > failed since NET_TYPE_HOSTDEV is not supported there. > Yep, understood. > I'll add this to commit message. Is that OK? > I fixed up the commit message and pushed the patch. Thanks Chunyan! Regards, Jim

1 0

[libvirt] [PATCH 1/2] libxl: support syntax <interface type="hostdev">
by Chunyan Liu 12 May '14

12 May '14

Signed-off-by: Chunyan Liu <cyliu(a)suse.com> --- src/libxl/libxl_conf.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 298c8a1..b7fed7f 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -921,25 +921,31 @@ static int libxlMakeNicList(virDomainDefPtr def, libxl_domain_config *d_config) { virDomainNetDefPtr *l_nics = def->nets; - int nnics = def->nnets; + size_t nnics = def->nnets; libxl_device_nic *x_nics; - size_t i; + size_t i, nvnics = 0; if (VIR_ALLOC_N(x_nics, nnics) < 0) return -1; for (i = 0; i < nnics; i++) { - if (libxlMakeNic(def, l_nics[i], &x_nics[i])) + if (l_nics[i]->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) + continue; + + if (libxlMakeNic(def, l_nics[i], &x_nics[nvnics])) goto error; /* * The devid (at least right now) will not get initialized by * libxl in the setup case but is required for starting the * device-model. */ - if (x_nics[i].devid < 0) - x_nics[i].devid = i; + if (x_nics[nvnics].devid < 0) + x_nics[nvnics].devid = nvnics; + + nvnics++; } + VIR_SHRINK_N(x_nics, nnics, nnics - nvnics); d_config->nics = x_nics; d_config->num_nics = nnics; -- 1.8.4.5

4 6