March 2011 - Devel - libvirt List Archives

[libvirt] [PATCH v2 0/4] Support domain snapshots with current QMP
by Jiri Denemark 08 Mar '11

08 Mar '11

This version 2 of this series implements the third alternative (suggested by Dan Berrange) for falling back to HMP if appropriate snapshot command does not exist in QMP. Version 2: - refactor text monitor commands into parts which can be called directly from json monitor code - implement HMP fallback for loadvm and delvm in addition to savevm Jiri Denemark (4): qemu: Refactor text monitor snapshot commands qemu: Fallback to HMP when savevm QMP command is not found qemu: Refactor qemuDomainSnapshotCreateXML qemu: Stop guest CPUs before creating a snapshot src/qemu/qemu_driver.c | 156 ++++++++++++++++++++++---------- src/qemu/qemu_monitor_json.c | 204 +++++++++++++++++++++++++++++++++++------- src/qemu/qemu_monitor_text.c | 179 ++++++++++++++++++++++++++----------- src/qemu/qemu_monitor_text.h | 8 ++ 4 files changed, 411 insertions(+), 136 deletions(-) -- 1.7.4.1

1 4

[libvirt] Add persistent XML for cpu tunables
by Osier Yang 08 Mar '11

08 Mar '11

These are pretty rough patches, something like a draft, I beleive there must be many problems, please review it heavily, :-) New XML: <cputune> <shares>2048</shares> <vcpupin vcpu='0' cpuset='0-4,^1'/> <vcpupin vcpu='1' cpuset='1,3'/> <vcpupin vcpu='2' cpuset='0,2'/> </cputune> "shares" is to define the the proportional weighted cpu share for the domain. "vcpupin" is to define the cpu affinities of vcpus, it will not be displayed if one doesn't specify it explicitly in XML or set the cpu affinites for vcpu via "vcpupin", means there will be no vcpupin element in domain XML by default, and the constraints are: - Error if one specify entries more than the count of maxvcpus. - Error when one specify entries for same vcpu. - Error if value of attribute "vcpu" is more than count of "maxvcpus - 1". Attribute "cpuset" works same as "cpuset" of element "vcpu", reuse the codes for parsing and formating value of "cpuset" of element "vcpu". NB, the idea to add persistent XML for "cpushares" is from "Nikunj A. Dadhania": https://www.redhat.com/archives/libvir-list/2011-January/msg01183.html I rebased it and include it together in this patch series. [PATCH 1/6] cputune: new tests for testing cputune xml [PATCH 2/6] cputune: support cputune for xend driver [PATCH 3/6] cputune: support cputune for qemu driver [PATCH 4/6] cputune: support cputune xml for LXC driver [PATCH 5/6] cputune: support cputune xml config [PATCH 6/6] cputune: Add docs xml schema for cputune xml

2 11

[libvirt] libvirt-php: various fixes
by Tiziano Mueller 08 Mar '11

08 Mar '11

While building a libvirt-php package for Gentoo Linux I had to workaround a couple of build issues. The first two patches are the results. The last patch is a fix for the case libvirt has been built without Xen support. In this case libvirt_connect (and phpinfo()) fails because virGetVersion is called with a non-NULL typeVer argument and libvirt therefore checks for Xen version. I guess the documentation for virGetVersion should be updated since it also returns 0 if both type and typeVer are NULL. [PATCH 1/3] Fix building with threaded php. [PATCH 2/3] add --with-php-config flag [PATCH 3/3] fix libvirt_connect failure without Xen

2 5

[libvirt] [PATCH] Fix performance problem of virStorageVolCreateXMLFrom()
by Minoru Usui 08 Mar '11

08 Mar '11

virStorageVolCreateXMLFrom() is slow if destination Pool is LVM. Because write block size is not appropriate. On linux environment, block size of LVM Pool which is made by virStoragePoolCreateXML() is 4096 bytes. On the other hand, write block size of virStorageVolCreateXMLFrom() is 512 bytes. This value are hard corded in virStorageBackendCopyToFD(). This patch fix this block size problem on linux. [lvm block size] # virsh vol-dumpxml vol.img --pool lvm-pool |grep '<path>' <path>/dev/lvm-pool/vol.img</path> # blockdev --getbsz /dev/lvm-pool/vol.img 4096 [before patch] # echo 1 > /proc/sys/vm/drop_caches # time virsh vol-create-from lvm-pool vol.xml --inputpool dir-pool vol.img real 1m17.736s user 0m0.007s sys 0m0.021s [after patch] # echo 1 > /proc/sys/vm/drop_caches # time virsh vol-create-from lvm-pool vol.xml --inputpool dir-pool vol.img real 0m36.883s user 0m0.011s sys 0m0.031s Signed-off-by: Minoru Usui <usui(a)mxm.nes.nec.co.jp> --- src/storage/storage_backend.c | 23 +++++++++++++++++++---- 1 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 174155d..a1a930c 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -36,6 +36,10 @@ #include <sys/stat.h> #include <sys/param.h> #include <dirent.h> +#ifdef __linux__ +#include <sys/ioctl.h> +#include <linux/fs.h> +#endif #if HAVE_SELINUX # include <selinux/selinux.h> @@ -108,6 +112,9 @@ enum { TOOL_QCOW_CREATE, }; +#define READ_BLOCK_SIZE_DEFAULT (1024 * 1024) +#define WRITE_BLOCK_SIZE_DEFAULT (512) + static int ATTRIBUTE_NONNULL (2) virStorageBackendCopyToFD(virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, @@ -119,8 +126,10 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, int amtread = -1; int ret = 0; unsigned long long remain; - size_t rbytes = 1024 * 1024; /* For Read */ - char zerobuf[512]; + size_t rbytes = READ_BLOCK_SIZE_DEFAULT; + size_t wbytes = WRITE_BLOCK_SIZE_DEFAULT; + int interval; + char zerobuf[WRITE_BLOCK_SIZE_DEFAULT]; char *buf = NULL; if ((inputfd = open(inputvol->target.path, O_RDONLY)) < 0) { @@ -131,6 +140,12 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, goto cleanup; } +#ifdef __linux__ + if (ioctl(fd, BLKBSZGET, &wbytes) < 0) { + wbytes = WRITE_BLOCK_SIZE_DEFAULT; + } +#endif + bzero(&zerobuf, sizeof(zerobuf)); if (VIR_ALLOC_N(buf, rbytes) < 0) { @@ -160,7 +175,7 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, * blocks */ amtleft = amtread; do { - int interval = ((512 > amtleft) ? amtleft : 512); + interval = ((wbytes > amtleft) ? amtleft : wbytes); int offset = amtread - amtleft; if (is_dest_file && memcmp(buf+offset, zerobuf, interval) == 0) { @@ -179,7 +194,7 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, goto cleanup; } - } while ((amtleft -= 512) > 0); + } while ((amtleft -= interval) > 0); } if (VIR_CLOSE(inputfd) < 0) { -- Minoru Usui <usui(a)mxm.nes.nec.co.jp>

2 2

[libvirt] [PATCH 0/3] more cgroup ACL audit improvements
by Eric Blake 08 Mar '11

08 Mar '11

Based on some feedback from Steve Grubb, Stephan Mueller, and others (unfortunately most of it on some non-public lists), I'm proposing the following patches to enhance my earlier audits for device cgroup ACLs. Pre-patch, cgroup audits looked like: type=VIRT_RESOURCE msg=audit(1298068194.479:83142): user pid=23863 uid=0 auid=500 ses=2 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 msg='resrc=cgroup reason=deny vm="fedora_12" uuid=51c6fc83-65a4-e627-b698-042b00145201 item=all: exe="/home/dummy/libvirt/daemon/.libs/lt-libvirtd" hostname=? addr=? terminal=pts/0 res=success' type=VIRT_RESOURCE msg=audit(1298068194.480:83143): user pid=23863 uid=0 auid=500 ses=2 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 msg='resrc=cgroup reason=allow vm="fedora_12" uuid=51c6fc83-65a4-e627-b698-042b00145201 item=major type="pty": exe="/home/dummy/libvirt/daemon/.libs/lt-libvirtd" hostname=? addr=? terminal=pts/0 res=success' type=VIRT_RESOURCE msg=audit(1298068194.480:83145): user pid=23863 uid=0 auid=500 ses=2 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 msg='resrc=cgroup reason=allow vm="fedora_12" uuid=51c6fc83-65a4-e627-b698-042b00145201 item=file path="/dev/null": exe="/home/dummy/libvirt/daemon/.libs/lt-libvirtd" hostname=? addr=? terminal=pts/0 res=success' Post-patch, the same three audits are modified to include cgroup controller, rdev information for files, major device number for categories, and better names so as not to collide with well-known audit field names (for example, audit libraries expect item= to match a decimal integer, so I used class= instead). type=VIRT_RESOURCE msg=audit(1299541864.111:78295): user pid=30632 uid=0 auid=500 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 msg='resrc=cgroup reason=deny vm="fedora_12" uuid=51c6fc83-65a4-e627-b698-042b00145201 cgroup="/cgroup/devices/libvirt/qemu/fedora_12/" class=all: exe="/home/dummy/libvirt/daemon/.libs/lt-libvirtd" hostname=? addr=? terminal=pts/0 res=success' type=VIRT_RESOURCE msg=audit(1299541864.112:78296): user pid=30632 uid=0 auid=500 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 msg='resrc=cgroup reason=allow vm="fedora_12" uuid=51c6fc83-65a4-e627-b698-042b00145201 cgroup="/cgroup/devices/libvirt/qemu/fedora_12/" class=major category=pty maj=88: exe="/home/dummy/libvirt/daemon/.libs/lt-libvirtd" hostname=? addr=? terminal=pts/0 res=success' type=VIRT_RESOURCE msg=audit(1299541864.112:78297): user pid=30632 uid=0 auid=500 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 msg='resrc=cgroup reason=allow vm="fedora_12" uuid=51c6fc83-65a4-e627-b698-042b00145201 cgroup="/cgroup/devices/libvirt/qemu/fedora_12/" class=path path=/dev/null rdev=01:03: exe="/home/dummy/libvirt/daemon/.libs/lt-libvirtd" hostname=? addr=? terminal=pts/0 res=success' Eric Blake (3): audit: tweak audit messages to match conventions audit: split cgroup audit types to allow more information audit: also audit cgroup controller path src/libvirt_private.syms | 1 + src/qemu/qemu_audit.c | 115 ++++++++++++++++++++++++++++++++++++++++------ src/qemu/qemu_audit.h | 14 +++++- src/qemu/qemu_cgroup.c | 29 ++++++------ src/qemu/qemu_driver.c | 8 ++-- src/util/cgroup.c | 8 ++-- src/util/cgroup.h | 5 ++ 7 files changed, 142 insertions(+), 38 deletions(-) -- 1.7.4

1 3

[libvirt] problem attaching/detaching disk
by Shi Jin 07 Mar '11

07 Mar '11

Hi there, Attaching/detachnig virtio disk to a VM used to work fine with RHEL-5 but with RHEL-6 it seems to have problems. Attaching seems to work. For example, I can do virsh attach-disk <vmname> <qcow2 file name> vdb --driver file --subdriver qcow2 However, it generates a bit weird XML: <disk type='file' device='disk'> <driver name='file' type='qcow2'/> <source file='<file>/> <target dev='vdb' bus='virtio'/> <alias name='virtio-disk1'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> </disk> In contrary, our original VM system disk has <disk type='file' device='disk'> <driver name='qemu' type='qcow2'/> <source file='<sys disk image>'/> <target dev='vda' bus='virtio'/> <alias name='virtio-disk0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </disk> Note the different of driver name: qemu vs file. I think qemu is the proper name to use. This has been mentioned last year on this list [1]. I wonder if there has already been a proper fix for this problem. More seriously about this is that I am not able to detach the just attached disk, most likely due to the problem of wrong driver name. [cloudadmin@test2 ~]$ virsh detach-disk one-52 vdb error: Failed to detach disk error: operation failed: detaching virtio-disk1 device failed: Device 'virtio-disk1' not found But if I tried to remove the vda which uses qemu as driver name it would work. [cloudadmin@test2 ~]$ virsh detach-disk one-52 vda Disk detached successfully I also tried with raw disks to attach/detach and saw the same problems. Thank you very much. Shi [1]: http://www.redhat.com/archives/libvir-list/2010-August/msg00449.html -- Shi Jin, PhD

4 14

[libvirt] [PATCH v2] virsh: Change option parsing functions to return tri-state information.
by Michal Privoznik 07 Mar '11

07 Mar '11

This is needed to detect situations when optional argument was specified with non-integer value: '--int-opt foo'. To keep functions uniform vshCommandOptString function was also changed, because it returns tri-state value as well. Given result pointer is updated only in case of success. If parsing fails, result is not updated at all. diff --git a/tools/virsh.c b/tools/virsh.c index f3754d7..c274a6b 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -252,13 +252,14 @@ static const vshCmdGrp *vshCmdGrpSearch(const char *grpname); static int 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 *found); -static unsigned long vshCommandOptUL(const vshCmd *cmd, const char *name, - int *found); -static char *vshCommandOptString(const vshCmd *cmd, const char *name, - int *found); -static long long vshCommandOptLongLong(const vshCmd *cmd, const char *name, - int *found); +static int vshCommandOptInt(const vshCmd *cmd, const char *name, int *value) + ATTRIBUTE_NONNULL(3); +static int vshCommandOptUL(const vshCmd *cmd, const char *name, + unsigned long *value) ATTRIBUTE_NONNULL(3); +static int vshCommandOptString(const vshCmd *cmd, const char *name, + char **value) ATTRIBUTE_NONNULL(3); +static int vshCommandOptLongLong(const vshCmd *cmd, const char *name, + unsigned long long *value) ATTRIBUTE_NONNULL(3); static int vshCommandOptBool(const vshCmd *cmd, const char *name); static char *vshCommandOptArgv(const vshCmd *cmd, int count); @@ -589,9 +590,9 @@ static const vshCmdOptDef opts_help[] = { static int cmdHelp(vshControl *ctl, const vshCmd *cmd) { - const char *name; + char *name = NULL; - name = vshCommandOptString(cmd, "command", NULL); + vshCommandOptString(cmd, "command", &name); if (!name) { const vshCmdGrp *grp; @@ -704,8 +705,7 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd) } VIR_FREE(ctl->name); - name = vshCommandOptString(cmd, "name", NULL); - if (!name) + if (vshCommandOptString(cmd, "name", &name) <= 0) return FALSE; ctl->name = vshStrdup(ctl, name); @@ -773,7 +773,7 @@ cmdConsole(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; int ret; - const char *name; + const char *name = NULL; if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; @@ -781,7 +781,7 @@ cmdConsole(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return FALSE; - name = vshCommandOptString(cmd, "devname", NULL); + vshCommandOptString(cmd, "devname", (char**) &name); ret = cmdRunConsole(ctl, dom, name); @@ -964,7 +964,7 @@ static int cmdDomblkstat (vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; - char *name, *device; + char *name, *device = NULL; struct _virDomainBlockStats stats; if (!vshConnectionUsability (ctl, ctl->conn)) @@ -973,7 +973,7 @@ cmdDomblkstat (vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain (ctl, cmd, &name))) return FALSE; - if (!(device = vshCommandOptString (cmd, "device", NULL))) { + if (vshCommandOptString (cmd, "device", &device) <= 0) { virDomainFree(dom); return FALSE; } @@ -1021,7 +1021,7 @@ static int cmdDomIfstat (vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; - char *name, *device; + char *name, *device = NULL; struct _virDomainInterfaceStats stats; if (!vshConnectionUsability (ctl, ctl->conn)) @@ -1030,7 +1030,7 @@ cmdDomIfstat (vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain (ctl, cmd, &name))) return FALSE; - if (!(device = vshCommandOptString (cmd, "interface", NULL))) { + if (vshCommandOptString (cmd, "interface", &device) <= 0) { virDomainFree(dom); return FALSE; } @@ -1144,7 +1144,7 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd) virDomainBlockInfo info; virDomainPtr dom; int ret = TRUE; - const char *device; + const char *device = NULL; if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; @@ -1152,7 +1152,7 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return FALSE; - if (!(device = vshCommandOptString (cmd, "device", NULL))) { + if (vshCommandOptString (cmd, "device", (char**) &device) <= 0) { virDomainFree(dom); return FALSE; } @@ -1230,8 +1230,7 @@ static int cmdCreate(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; - char *from; - int found; + char *from = NULL; int ret = TRUE; char *buffer; #ifndef WIN32 @@ -1242,8 +1241,7 @@ cmdCreate(vshControl *ctl, const vshCmd *cmd) if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; - from = vshCommandOptString(cmd, "file", &found); - if (!found) + if (vshCommandOptString(cmd, "file", &from) <= 0) return FALSE; if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) @@ -1288,16 +1286,14 @@ static int cmdDefine(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; - char *from; - int found; + char *from = NULL; int ret = TRUE; char *buffer; if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; - from = vshCommandOptString(cmd, "file", &found); - if (!found) + if (vshCommandOptString(cmd, "file", &from) <= 0) return FALSE; if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) @@ -1336,15 +1332,13 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; int ret = TRUE; - char *name; - int found; + char *name = NULL; int id; if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; - name = vshCommandOptString(cmd, "domain", &found); - if (!found) + if (vshCommandOptString(cmd, "domain", &name) <= 0) return FALSE; if (name && virStrToLong_i(name, NULL, 10, &id) == 0 @@ -1455,13 +1449,13 @@ cmdSave(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; char *name; - char *to; + char *to = NULL; int ret = TRUE; if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; - if (!(to = vshCommandOptString(cmd, "file", NULL))) + if (vshCommandOptString(cmd, "file", &to) <= 0) return FALSE; if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) @@ -1594,7 +1588,6 @@ static int cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd, virSchedParameterPtr param) { - int found; char *data; /* Legacy 'weight' parameter */ @@ -1602,8 +1595,7 @@ cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd, param->type == VIR_DOMAIN_SCHED_FIELD_UINT && vshCommandOptBool(cmd, "weight")) { int val; - val = vshCommandOptInt(cmd, "weight", &found); - if (!found) { + if (vshCommandOptInt(cmd, "weight", &val) <= 0) { vshError(ctl, "%s", _("Invalid value of weight")); return -1; } else { @@ -1617,8 +1609,7 @@ cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd, param->type == VIR_DOMAIN_SCHED_FIELD_UINT && vshCommandOptBool(cmd, "cap")) { int val; - val = vshCommandOptInt(cmd, "cap", &found); - if (!found) { + if (vshCommandOptInt(cmd, "cap", &val) <= 0) { vshError(ctl, "%s", _("Invalid value of cap")); return -1; } else { @@ -1627,7 +1618,7 @@ cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd, return 1; } - if ((data = vshCommandOptString(cmd, "set", NULL))) { + if (vshCommandOptString(cmd, "set", &data)) { char *val = strchr(data, '='); int match = 0; if (!val) { @@ -1746,8 +1737,8 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) /* See if we've tried to --set var=val. If so, the fact that we reach this point (with update == 0) means that "var" did not match any of the settable parameters. Report the error. */ - char *var_value_pair = vshCommandOptString(cmd, "set", NULL); - if (var_value_pair) { + char *var_value_pair = NULL; + if (vshCommandOptString(cmd, "set", &var_value_pair) > 0) { vshError(ctl, _("invalid scheduler option: %s"), var_value_pair); goto cleanup; @@ -1804,15 +1795,13 @@ static const vshCmdOptDef opts_restore[] = { static int cmdRestore(vshControl *ctl, const vshCmd *cmd) { - char *from; - int found; + char *from = NULL; int ret = TRUE; if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; - from = vshCommandOptString(cmd, "file", &found); - if (!found) + if (vshCommandOptString(cmd, "file", &from) <= 0) return FALSE; if (virDomainRestore(ctl->conn, from) == 0) { @@ -1846,14 +1835,14 @@ cmdDump(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; char *name; - char *to; + char *to = NULL; int ret = TRUE; int flags = 0; if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; - if (!(to = vshCommandOptString(cmd, "file", NULL))) + if (vshCommandOptString(cmd, "file", &to) <= 0) return FALSE; if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) @@ -2300,7 +2289,10 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd) if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; - cell = vshCommandOptInt(cmd, "cellno", &cell_given); + if ( (cell_given = vshCommandOptInt(cmd, "cellno", &cell)) < 0) { + vshError(ctl, "%s", _("cell number has to be a number")); + goto cleanup; + } all_given = vshCommandOptBool(cmd, "all"); if (all_given && cell_given) { @@ -2412,10 +2404,10 @@ static const vshCmdOptDef opts_maxvcpus[] = { static int cmdMaxvcpus(vshControl *ctl, const vshCmd *cmd) { - char *type; + char *type = NULL; int vcpus; - type = vshCommandOptString(cmd, "type", NULL); + vshCommandOptString(cmd, "type", &type); if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; @@ -2712,9 +2704,8 @@ cmdVcpupin(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom; virNodeInfo nodeinfo; int vcpu; - char *cpulist; + char *cpulist = NULL; int ret = TRUE; - int vcpufound = 0; unsigned char *cpumap; int cpumaplen; int i; @@ -2726,14 +2717,13 @@ cmdVcpupin(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return FALSE; - vcpu = vshCommandOptInt(cmd, "vcpu", &vcpufound); - if (!vcpufound) { + if (vshCommandOptInt(cmd, "vcpu", &vcpu) <= 0) { vshError(ctl, "%s", _("vcpupin: Invalid or missing vCPU number.")); virDomainFree(dom); return FALSE; } - if (!(cpulist = vshCommandOptString(cmd, "cpulist", NULL))) { + if (vshCommandOptString(cmd, "cpulist", &cpulist) <= 0) { virDomainFree(dom); return FALSE; } @@ -2862,7 +2852,7 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return FALSE; - count = vshCommandOptInt(cmd, "count", &count); + vshCommandOptInt(cmd, "count", &count); if (!flags) { if (virDomainSetVcpus(dom, count) != 0) { @@ -2918,7 +2908,7 @@ cmdSetmem(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; virDomainInfo info; - unsigned long kilobytes; + unsigned long kilobytes = 0; int ret = TRUE; if (!vshConnectionUsability(ctl, ctl->conn)) @@ -2927,7 +2917,7 @@ cmdSetmem(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return FALSE; - kilobytes = vshCommandOptUL(cmd, "kilobytes", NULL); + vshCommandOptUL(cmd, "kilobytes", &kilobytes); if (kilobytes <= 0) { virDomainFree(dom); vshError(ctl, _("Invalid value of %lu for memory size"), kilobytes); @@ -2984,7 +2974,7 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return FALSE; - kilobytes = vshCommandOptInt(cmd, "kilobytes", &kilobytes); + vshCommandOptInt(cmd, "kilobytes", &kilobytes); if (kilobytes <= 0) { virDomainFree(dom); vshError(ctl, _("Invalid value of %d for memory size"), kilobytes); @@ -3043,7 +3033,7 @@ static int cmdMemtune(vshControl * ctl, const vshCmd * cmd) { virDomainPtr dom; - long long hard_limit, soft_limit, swap_hard_limit, min_guarantee; + long long hard_limit = 0, soft_limit = 0, swap_hard_limit = 0, min_guarantee = 0; int nparams = 0; unsigned int i = 0; virMemoryParameterPtr params = NULL, temp = NULL; @@ -3055,23 +3045,19 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return FALSE; - hard_limit = - vshCommandOptLongLong(cmd, "hard-limit", NULL); + vshCommandOptLongLong(cmd, "hard-limit", (unsigned long long*) &hard_limit); if (hard_limit) nparams++; - soft_limit = - vshCommandOptLongLong(cmd, "soft-limit", NULL); + vshCommandOptLongLong(cmd, "soft-limit", (unsigned long long*) &soft_limit); if (soft_limit) nparams++; - swap_hard_limit = - vshCommandOptLongLong(cmd, "swap-hard-limit", NULL); + vshCommandOptLongLong(cmd, "swap-hard-limit", (unsigned long long*) &swap_hard_limit); if (swap_hard_limit) nparams++; - min_guarantee = - vshCommandOptLongLong(cmd, "min-guarantee", NULL); + vshCommandOptLongLong(cmd, "min-guarantee", (unsigned long long*)&min_guarantee); if (min_guarantee) nparams++; @@ -3315,8 +3301,8 @@ static int cmdDomXMLFromNative(vshControl *ctl, const vshCmd *cmd) { int ret = TRUE; - char *format; - char *configFile; + char *format = NULL; + char *configFile = NULL; char *configData; char *xmlData; int flags = 0; @@ -3324,8 +3310,8 @@ cmdDomXMLFromNative(vshControl *ctl, const vshCmd *cmd) if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; - format = vshCommandOptString(cmd, "format", NULL); - configFile = vshCommandOptString(cmd, "config", NULL); + vshCommandOptString(cmd, "format",&format); + vshCommandOptString(cmd, "config", &configFile); if (virFileReadAll(configFile, 1024*1024, &configData) < 0) return FALSE; @@ -3360,8 +3346,8 @@ static int cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd) { int ret = TRUE; - char *format; - char *xmlFile; + char *format = NULL; + char *xmlFile = NULL; char *configData; char *xmlData; int flags = 0; @@ -3369,8 +3355,8 @@ cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd) if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; - format = vshCommandOptString(cmd, "format", NULL); - xmlFile = vshCommandOptString(cmd, "xml", NULL); + vshCommandOptString(cmd, "format", &format); + vshCommandOptString(cmd, "xml", &xmlFile); if (virFileReadAll(xmlFile, 1024*1024, &xmlData) < 0) return FALSE; @@ -3525,10 +3511,10 @@ doMigrate (void *opaque) { char ret = '1'; virDomainPtr dom = NULL; - const char *desturi; - const char *migrateuri; - const char *dname; - int flags = 0, found; + const char *desturi = NULL; + const char *migrateuri = NULL; + const char *dname = NULL; + int flags = 0; vshCtrlData *data = opaque; vshControl *ctl = data->ctl; const vshCmd *cmd = data->cmd; @@ -3547,13 +3533,12 @@ doMigrate (void *opaque) if (!(dom = vshCommandOptDomain (ctl, cmd, NULL))) goto out; - desturi = vshCommandOptString (cmd, "desturi", &found); - if (!found) + if (vshCommandOptString (cmd, "desturi", (char**) &desturi) <= 0) goto out; - migrateuri = vshCommandOptString (cmd, "migrateuri", NULL); + vshCommandOptString(cmd, "migrateuri", (char**) &migrateuri); - dname = vshCommandOptString (cmd, "dname", NULL); + vshCommandOptString(cmd, "dname", (char**) &dname); if (vshCommandOptBool (cmd, "live")) flags |= VIR_MIGRATE_LIVE; @@ -3645,7 +3630,6 @@ cmdMigrate (vshControl *ctl, const vshCmd *cmd) int ret = -1; virThread workerThread; struct pollfd pollfd; - int found; char retchar; struct sigaction sig_action; struct sigaction old_sig_action; @@ -3670,8 +3654,7 @@ cmdMigrate (vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool (cmd, "live")) live_flag = TRUE; - timeout = vshCommandOptInt(cmd, "timeout", &found); - if (found) { + if (vshCommandOptInt(cmd, "timeout", &timeout)) { if (! live_flag) { vshError(ctl, "%s", _("migrate: Unexpected timeout for offline migration")); goto cleanup; @@ -3798,7 +3781,6 @@ cmdMigrateSetMaxDowntime(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom = NULL; long long downtime; - int found; int ret = FALSE; if (!vshConnectionUsability(ctl, ctl->conn)) @@ -3807,8 +3789,8 @@ cmdMigrateSetMaxDowntime(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return FALSE; - downtime = vshCommandOptLongLong(cmd, "downtime", &found); - if (!found || downtime < 1) { + if ((vshCommandOptLongLong(cmd, "downtime", (unsigned long long*)&downtime) <= 0) || + (downtime < 0)) { vshError(ctl, "%s", _("migrate: Invalid downtime")); goto done; } @@ -3890,16 +3872,14 @@ static int cmdNetworkCreate(vshControl *ctl, const vshCmd *cmd) { virNetworkPtr network; - char *from; - int found; + char *from = NULL; int ret = TRUE; char *buffer; if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; - from = vshCommandOptString(cmd, "file", &found); - if (!found) + if (vshCommandOptString(cmd, "file", &from) <= 0) return FALSE; if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) @@ -3938,16 +3918,14 @@ static int cmdNetworkDefine(vshControl *ctl, const vshCmd *cmd) { virNetworkPtr network; - char *from; - int found; + char *from = NULL; int ret = TRUE; char *buffer; if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; - from = vshCommandOptString(cmd, "file", &found); - if (!found) + if (vshCommandOptString(cmd, "file", &from) <= 0) return FALSE; if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) @@ -4711,16 +4689,14 @@ static int cmdInterfaceDefine(vshControl *ctl, const vshCmd *cmd) { virInterfacePtr iface; - char *from; - int found; + char *from = NULL; int ret = TRUE; char *buffer; if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; - from = vshCommandOptString(cmd, "file", &found); - if (!found) + if (vshCommandOptString(cmd, "file", &from) <= 0) return FALSE; if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) @@ -4873,16 +4849,14 @@ static int cmdNWFilterDefine(vshControl *ctl, const vshCmd *cmd) { virNWFilterPtr nwfilter; - char *from; - int found; + char *from = NULL; int ret = TRUE; char *buffer; if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; - from = vshCommandOptString(cmd, "file", &found); - if (!found) + if (vshCommandOptString(cmd, "file", &from) <= 0) return FALSE; if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) @@ -5214,16 +5188,14 @@ static int cmdPoolCreate(vshControl *ctl, const vshCmd *cmd) { virStoragePoolPtr pool; - char *from; - int found; + char *from = NULL; int ret = TRUE; char *buffer; if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; - from = vshCommandOptString(cmd, "file", &found); - if (!found) + if (vshCommandOptString(cmd, "file", &from) <= 0) return FALSE; if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) @@ -5266,18 +5238,15 @@ static int cmdNodeDeviceCreate(vshControl *ctl, const vshCmd *cmd) { virNodeDevicePtr dev = NULL; - char *from; - int found = 0; + char *from = NULL; int ret = TRUE; char *buffer; if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; - from = vshCommandOptString(cmd, "file", &found); - if (!found) { + if (vshCommandOptString(cmd, "file", &from) <= 0) return FALSE; - } if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) return FALSE; @@ -5319,17 +5288,14 @@ cmdNodeDeviceDestroy(vshControl *ctl, const vshCmd *cmd) { virNodeDevicePtr dev = NULL; int ret = TRUE; - int found = 0; - char *name; + char *name = NULL; if (!vshConnectionUsability(ctl, ctl->conn)) { return FALSE; } - name = vshCommandOptString(cmd, "name", &found); - if (!found) { + if (vshCommandOptString(cmd, "name", &name) <= 0) return FALSE; - } dev = virNodeDeviceLookupByName(ctl->conn, name); @@ -5363,23 +5329,21 @@ static const vshCmdOptDef opts_pool_X_as[] = { static int buildPoolXML(const vshCmd *cmd, char **retname, char **xml) { - int found; - char *name, *type, *srcHost, *srcPath, *srcDev, *srcName, *srcFormat, *target; + char *name = NULL, *type = NULL, *srcHost = NULL, *srcPath = NULL, + *srcDev = NULL, *srcName = NULL, *srcFormat = NULL, *target = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; - name = vshCommandOptString(cmd, "name", &found); - if (!found) + if (vshCommandOptString(cmd, "name", &name) <= 0) goto cleanup; - type = vshCommandOptString(cmd, "type", &found); - if (!found) + if (vshCommandOptString(cmd, "type", &type) <= 0) goto cleanup; - srcHost = vshCommandOptString(cmd, "source-host", &found); - srcPath = vshCommandOptString(cmd, "source-path", &found); - srcDev = vshCommandOptString(cmd, "source-dev", &found); - srcName = vshCommandOptString(cmd, "source-name", &found); - srcFormat = vshCommandOptString(cmd, "source-format", &found); - target = vshCommandOptString(cmd, "target", &found); + vshCommandOptString(cmd, "source-host", &srcHost); + vshCommandOptString(cmd, "source-path", &srcPath); + vshCommandOptString(cmd, "source-dev", &srcDev); + vshCommandOptString(cmd, "source-name", &srcName); + vshCommandOptString(cmd, "source-format", &srcFormat); + vshCommandOptString(cmd, "target", &target); virBufferVSprintf(&buf, "<pool type='%s'>\n", type); virBufferVSprintf(&buf, " <name>%s</name>\n", name); @@ -5479,16 +5443,14 @@ static int cmdPoolDefine(vshControl *ctl, const vshCmd *cmd) { virStoragePoolPtr pool; - char *from; - int found; + char *from = NULL; int ret = TRUE; char *buffer; if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; - from = vshCommandOptString(cmd, "file", &found); - if (!found) + if (vshCommandOptString(cmd, "file", &from) <= 0) return FALSE; if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) @@ -6176,29 +6138,22 @@ static const vshCmdOptDef opts_find_storage_pool_sources_as[] = { static int cmdPoolDiscoverSourcesAs(vshControl * ctl, const vshCmd * cmd ATTRIBUTE_UNUSED) { - char *type, *host; + char *type = NULL, *host = NULL; char *srcSpec = NULL; char *srcList; - char *initiator; - int found; + char *initiator = NULL; - type = vshCommandOptString(cmd, "type", &found); - if (!found) + if (vshCommandOptString(cmd, "type", &type) <= 0) return FALSE; - host = vshCommandOptString(cmd, "host", &found); - if (!found) - host = NULL; - initiator = vshCommandOptString(cmd, "initiator", &found); - if (!found) - initiator = NULL; + vshCommandOptString(cmd, "host", &host); + vshCommandOptString(cmd, "initiator", &initiator); if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; if (host) { - char *port = vshCommandOptString(cmd, "port", &found); - if (!found) - port = NULL; + char *port = NULL; + vshCommandOptString(cmd, "port", &port); virBuffer buf = VIR_BUFFER_INITIALIZER; virBufferAddLit(&buf, "<source>\n"); virBufferVSprintf(&buf, " <host name='%s'", host); @@ -6251,16 +6206,12 @@ static const vshCmdOptDef opts_find_storage_pool_sources[] = { static int cmdPoolDiscoverSources(vshControl * ctl, const vshCmd * cmd ATTRIBUTE_UNUSED) { - char *type, *srcSpecFile, *srcList; + char *type = NULL, *srcSpecFile = NULL, *srcList; char *srcSpec = NULL; - int found; - type = vshCommandOptString(cmd, "type", &found); - if (!found) + if ( vshCommandOptString(cmd, "type", &type) <= 0) return FALSE; - srcSpecFile = vshCommandOptString(cmd, "srcSpec", &found); - if (!found) - srcSpecFile = NULL; + vshCommandOptString(cmd, "srcSpec", &srcSpecFile); if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; @@ -6501,9 +6452,8 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd) { virStoragePoolPtr pool; virStorageVolPtr vol; - int found; char *xml; - char *name, *capacityStr, *allocationStr, *format; + char *name = NULL, *capacityStr = NULL, *allocationStr = NULL, *format = NULL; char *snapshotStrVol, *snapshotStrFormat; unsigned long long capacity, allocation = 0; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -6515,24 +6465,22 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd) VSH_BYNAME))) return FALSE; - name = vshCommandOptString(cmd, "name", &found); - if (!found) + if (vshCommandOptString(cmd, "name", &name) <= 0) goto cleanup; - capacityStr = vshCommandOptString(cmd, "capacity", &found); - if (!found) + if (vshCommandOptString(cmd, "capacity", &capacityStr) <= 0) goto cleanup; if (cmdVolSize(capacityStr, &capacity) < 0) vshError(ctl, _("Malformed size %s"), capacityStr); - allocationStr = vshCommandOptString(cmd, "allocation", &found); + vshCommandOptString(cmd, "allocation", &allocationStr); if (allocationStr && cmdVolSize(allocationStr, &allocation) < 0) vshError(ctl, _("Malformed size %s"), allocationStr); - format = vshCommandOptString(cmd, "format", &found); - snapshotStrVol = vshCommandOptString(cmd, "backing-vol", &found); - snapshotStrFormat = vshCommandOptString(cmd, "backing-vol-format", &found); + vshCommandOptString(cmd, "format", &format); + vshCommandOptString(cmd, "backing-vol", &snapshotStrVol); + vshCommandOptString(cmd, "backing-vol-format", &snapshotStrFormat); virBufferAddLit(&buf, "<volume>\n"); virBufferVSprintf(&buf, " <name>%s</name>\n", name); @@ -6723,8 +6671,7 @@ cmdVolCreate(vshControl *ctl, const vshCmd *cmd) { virStoragePoolPtr pool; virStorageVolPtr vol; - char *from; - int found; + char *from = NULL; int ret = TRUE; char *buffer; @@ -6735,8 +6682,7 @@ cmdVolCreate(vshControl *ctl, const vshCmd *cmd) VSH_BYNAME))) return FALSE; - from = vshCommandOptString(cmd, "file", &found); - if (!found) { + if (vshCommandOptString(cmd, "file", &from) <= 0) { virStoragePoolFree(pool); return FALSE; } @@ -6784,8 +6730,7 @@ cmdVolCreateFrom(vshControl *ctl, const vshCmd *cmd) { virStoragePoolPtr pool = NULL; virStorageVolPtr newvol = NULL, inputvol = NULL; - char *from; - int found; + char *from = NULL; int ret = FALSE; char *buffer = NULL; @@ -6795,10 +6740,8 @@ cmdVolCreateFrom(vshControl *ctl, const vshCmd *cmd) if (!(pool = vshCommandOptPoolBy(ctl, cmd, "pool", NULL, VSH_BYNAME))) goto cleanup; - from = vshCommandOptString(cmd, "file", &found); - if (!found) { + if (vshCommandOptString(cmd, "file", &from) <= 0) goto cleanup; - } if (!(inputvol = vshCommandOptVol(ctl, cmd, "vol", "inputpool", NULL))) goto cleanup; @@ -6883,9 +6826,8 @@ cmdVolClone(vshControl *ctl, const vshCmd *cmd) { virStoragePoolPtr origpool = NULL; virStorageVolPtr origvol = NULL, newvol = NULL; - char *name, *origxml = NULL; + char *name = NULL, *origxml = NULL; xmlChar *newxml = NULL; - int found; int ret = FALSE; if (!vshConnectionUsability(ctl, ctl->conn)) @@ -6900,8 +6842,7 @@ cmdVolClone(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - name = vshCommandOptString(cmd, "newname", &found); - if (!found) + if (vshCommandOptString(cmd, "newname", &name) <= 0) goto cleanup; origxml = virStorageVolGetXMLDesc(origvol, 0); @@ -7569,15 +7510,14 @@ static const vshCmdOptDef opts_secret_define[] = { static int cmdSecretDefine(vshControl *ctl, const vshCmd *cmd) { - char *from, *buffer; + char *from = NULL, *buffer; virSecretPtr res; char uuid[VIR_UUID_STRING_BUFLEN]; if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; - from = vshCommandOptString(cmd, "file", NULL); - if (!from) + if (vshCommandOptString(cmd, "file", &from) <= 0) return FALSE; if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) @@ -7660,8 +7600,8 @@ cmdSecretSetValue(vshControl *ctl, const vshCmd *cmd) { virSecretPtr secret; size_t value_size; - char *base64, *value; - int found, res, ret = FALSE; + char *base64 = NULL, *value; + int res, ret = FALSE; if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; @@ -7670,8 +7610,7 @@ cmdSecretSetValue(vshControl *ctl, const vshCmd *cmd) if (secret == NULL) return FALSE; - base64 = vshCommandOptString(cmd, "base64", &found); - if (!base64) + if (vshCommandOptString(cmd, "base64", &base64) <= 0) goto cleanup; if (!base64_decode_alloc(base64, strlen(base64), &value, &value_size)) { @@ -8036,17 +7975,15 @@ cmdNodeListDevicesPrint(vshControl *ctl, static int cmdNodeListDevices (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { - char *cap; + char *cap = NULL; char **devices; - int found, num_devices, i; + int num_devices, i; int tree = vshCommandOptBool(cmd, "tree"); if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; - cap = vshCommandOptString(cmd, "cap", &found); - if (!found) - cap = NULL; + vshCommandOptString(cmd, "cap", &cap); num_devices = virNodeNumOfDevices(ctl->conn, cap, 0); if (num_devices < 0) { @@ -8124,13 +8061,13 @@ static const vshCmdOptDef opts_node_device_dumpxml[] = { static int cmdNodeDeviceDumpXML (vshControl *ctl, const vshCmd *cmd) { - const char *name; + const char *name = NULL; virNodeDevicePtr device; char *xml; if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; - if (!(name = vshCommandOptString(cmd, "device", NULL))) + if (vshCommandOptString(cmd, "device", (char**) &name) <= 0) return FALSE; if (!(device = virNodeDeviceLookupByName(ctl->conn, name))) { vshError(ctl, "%s '%s'", _("Could not find matching device"), name); @@ -8167,13 +8104,13 @@ static const vshCmdOptDef opts_node_device_dettach[] = { static int cmdNodeDeviceDettach (vshControl *ctl, const vshCmd *cmd) { - const char *name; + const char *name = NULL; virNodeDevicePtr device; int ret = TRUE; if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; - if (!(name = vshCommandOptString(cmd, "device", NULL))) + if (vshCommandOptString(cmd, "device", (char**) &name) <= 0) return FALSE; if (!(device = virNodeDeviceLookupByName(ctl->conn, name))) { vshError(ctl, "%s '%s'", _("Could not find matching device"), name); @@ -8208,13 +8145,13 @@ static const vshCmdOptDef opts_node_device_reattach[] = { static int cmdNodeDeviceReAttach (vshControl *ctl, const vshCmd *cmd) { - const char *name; + const char *name = NULL; virNodeDevicePtr device; int ret = TRUE; if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; - if (!(name = vshCommandOptString(cmd, "device", NULL))) + if (vshCommandOptString(cmd, "device", (char**) &name) <= 0) return FALSE; if (!(device = virNodeDeviceLookupByName(ctl->conn, name))) { vshError(ctl, "%s '%s'", _("Could not find matching device"), name); @@ -8249,13 +8186,13 @@ static const vshCmdOptDef opts_node_device_reset[] = { static int cmdNodeDeviceReset (vshControl *ctl, const vshCmd *cmd) { - const char *name; + const char *name = NULL; virNodeDevicePtr device; int ret = TRUE; if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; - if (!(name = vshCommandOptString(cmd, "device", NULL))) + if (vshCommandOptString(cmd, "device", (char**) &name) <= 0) return FALSE; if (!(device = virNodeDeviceLookupByName(ctl->conn, name))) { vshError(ctl, "%s '%s'", _("Could not find matching device"), name); @@ -8516,10 +8453,9 @@ static int cmdAttachDevice(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; - char *from; + char *from = NULL; char *buffer; int ret = TRUE; - int found; unsigned int flags; if (!vshConnectionUsability(ctl, ctl->conn)) @@ -8528,8 +8464,7 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return FALSE; - from = vshCommandOptString(cmd, "file", &found); - if (!found) { + if (vshCommandOptString(cmd, "file", &from) <= 0) { virDomainFree(dom); return FALSE; } @@ -8586,7 +8521,6 @@ cmdDetachDevice(vshControl *ctl, const vshCmd *cmd) char *from; char *buffer; int ret = TRUE; - int found; unsigned int flags; if (!vshConnectionUsability(ctl, ctl->conn)) @@ -8595,8 +8529,7 @@ cmdDetachDevice(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return FALSE; - from = vshCommandOptString(cmd, "file", &found); - if (!found) { + if (vshCommandOptString(cmd, "file", &from) <= 0) { virDomainFree(dom); return FALSE; } @@ -8651,10 +8584,9 @@ static int cmdUpdateDevice(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; - char *from; + char *from = NULL; char *buffer; int ret = TRUE; - int found; unsigned int flags; if (!vshConnectionUsability(ctl, ctl->conn)) @@ -8663,8 +8595,7 @@ cmdUpdateDevice(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return FALSE; - from = vshCommandOptString(cmd, "file", &found); - if (!found) { + if (vshCommandOptString(cmd, "file", &from) <= 0) { virDomainFree(dom); return FALSE; } @@ -8727,7 +8658,8 @@ static int cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom = NULL; - char *mac, *target, *script, *type, *source, *model; + char *mac = NULL, *target = NULL, *script = NULL, *type = NULL, + *source = NULL, *model = NULL; int typ, ret = FALSE; unsigned int flags; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -8739,14 +8671,14 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) goto cleanup; - if (!(type = vshCommandOptString(cmd, "type", NULL))) + if (vshCommandOptString(cmd, "type", &type) <= 0) goto cleanup; - source = vshCommandOptString(cmd, "source", NULL); - target = vshCommandOptString(cmd, "target", NULL); - mac = vshCommandOptString(cmd, "mac", NULL); - script = vshCommandOptString(cmd, "script", NULL); - model = vshCommandOptString(cmd, "model", NULL); + vshCommandOptString(cmd, "source", &source); + vshCommandOptString(cmd, "target", &target); + vshCommandOptString(cmd, "mac", &mac); + vshCommandOptString(cmd, "script",&script); + vshCommandOptString(cmd, "model", &model); /* check interface type */ if (STREQ(type, "network")) { @@ -8848,10 +8780,10 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) goto cleanup; - if (!(type = vshCommandOptString(cmd, "type", NULL))) + if (vshCommandOptString(cmd, "type", &type) <= 0) goto cleanup; - mac = vshCommandOptString(cmd, "mac", NULL); + vshCommandOptString(cmd, "mac", &mac); doc = virDomainGetXMLDesc(dom, 0); if (!doc) @@ -8976,7 +8908,8 @@ static int cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom = NULL; - char *source, *target, *driver, *subdriver, *type, *mode; + char *source = NULL, *target = NULL, *driver = NULL, *subdriver = NULL, + *type = NULL, *mode = NULL; int isFile = 0, ret = FALSE; unsigned int flags; char *stype; @@ -8989,17 +8922,17 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) goto cleanup; - if (!(source = vshCommandOptString(cmd, "source", NULL))) + if (vshCommandOptString(cmd, "source", &source) <= 0) goto cleanup; - if (!(target = vshCommandOptString(cmd, "target", NULL))) + if (vshCommandOptString(cmd, "target", &target) <=0) goto cleanup; - driver = vshCommandOptString(cmd, "driver", NULL); - subdriver = vshCommandOptString(cmd, "subdriver", NULL); - type = vshCommandOptString(cmd, "type", NULL); - mode = vshCommandOptString(cmd, "mode", NULL); - stype = vshCommandOptString(cmd, "sourcetype", NULL); + vshCommandOptString(cmd, "driver", &driver); + vshCommandOptString(cmd, "subdriver", &subdriver); + vshCommandOptString(cmd, "type", &type); + vshCommandOptString(cmd, "mode", &mode); + vshCommandOptString(cmd, "sourcetype", &stype); if (!stype) { if (driver && (STREQ(driver, "file") || STREQ(driver, "tap"))) @@ -9104,7 +9037,7 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd) xmlNodePtr cur = NULL; xmlBufferPtr xml_buf = NULL; virDomainPtr dom = NULL; - char *doc, *target; + char *doc, *target = NULL; int i = 0, diff_tgt, ret = FALSE; unsigned int flags; @@ -9114,7 +9047,7 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) goto cleanup; - if (!(target = vshCommandOptString(cmd, "target", NULL))) + if (vshCommandOptString(cmd, "target", &target) <= 0) goto cleanup; doc = virDomainGetXMLDesc(dom, 0); @@ -9221,8 +9154,7 @@ static const vshCmdOptDef opts_cpu_compare[] = { static int cmdCPUCompare(vshControl *ctl, const vshCmd *cmd) { - char *from; - int found; + char *from = NULL; int ret = TRUE; char *buffer; int result; @@ -9230,8 +9162,7 @@ cmdCPUCompare(vshControl *ctl, const vshCmd *cmd) if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; - from = vshCommandOptString(cmd, "file", &found); - if (!found) + if (vshCommandOptString(cmd, "file", &from) <= 0) return FALSE; if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) @@ -9285,8 +9216,7 @@ static const vshCmdOptDef opts_cpu_baseline[] = { static int cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) { - char *from; - int found; + char *from = NULL; int ret = TRUE; char *buffer; char *result = NULL; @@ -9303,8 +9233,7 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; - from = vshCommandOptString(cmd, "file", &found); - if (!found) + if (vshCommandOptString(cmd, "file", &from) <= 0) return FALSE; if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) @@ -9516,8 +9445,7 @@ static const vshCmdOptDef opts_cd[] = { static int cmdCd(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { - const char *dir; - int found; + const char *dir = NULL; int ret = TRUE; bool dir_malloced = false; @@ -9526,8 +9454,7 @@ cmdCd(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) return FALSE; } - dir = vshCommandOptString(cmd, "dir", &found); - if (!found) { + if (vshCommandOptString(cmd, "dir", (char**) &dir) == 0) { uid_t uid = geteuid(); dir = virGetUserDirectory(uid); dir_malloced = !!dir; @@ -9830,7 +9757,7 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom = NULL; int ret = FALSE; - char *from; + char *from = NULL; char *buffer = NULL; virDomainSnapshotPtr snapshot = NULL; xmlDocPtr xml = NULL; @@ -9845,8 +9772,7 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd) if (dom == NULL) goto cleanup; - from = vshCommandOptString(cmd, "xmlfile", NULL); - if (from == NULL) + if (vshCommandOptString(cmd, "xmlfile", &from) <= 0) buffer = vshStrdup(ctl, "<domainsnapshot/>"); else { if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) { @@ -10103,7 +10029,7 @@ cmdSnapshotDumpXML(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom = NULL; int ret = FALSE; - char *name; + char *name = NULL; virDomainSnapshotPtr snapshot = NULL; char *xml = NULL; @@ -10114,8 +10040,7 @@ cmdSnapshotDumpXML(vshControl *ctl, const vshCmd *cmd) if (dom == NULL) goto cleanup; - name = vshCommandOptString(cmd, "snapshotname", NULL); - if (name == NULL) + if (vshCommandOptString(cmd, "snapshotname", &name) <= 0) goto cleanup; snapshot = virDomainSnapshotLookupByName(dom, name, 0); @@ -10160,7 +10085,7 @@ cmdDomainSnapshotRevert(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom = NULL; int ret = FALSE; - char *name; + char *name = NULL; virDomainSnapshotPtr snapshot = NULL; if (!vshConnectionUsability(ctl, ctl->conn)) @@ -10170,8 +10095,7 @@ cmdDomainSnapshotRevert(vshControl *ctl, const vshCmd *cmd) if (dom == NULL) goto cleanup; - name = vshCommandOptString(cmd, "snapshotname", NULL); - if (name == NULL) + if (vshCommandOptString(cmd, "snapshotname", &name) <= 0) goto cleanup; snapshot = virDomainSnapshotLookupByName(dom, name, 0); @@ -10213,7 +10137,7 @@ cmdSnapshotDelete(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom = NULL; int ret = FALSE; - char *name; + char *name = NULL; virDomainSnapshotPtr snapshot = NULL; unsigned int flags = 0; @@ -10224,8 +10148,7 @@ cmdSnapshotDelete(vshControl *ctl, const vshCmd *cmd) if (dom == NULL) goto cleanup; - name = vshCommandOptString(cmd, "snapshotname", NULL); - if (name == NULL) + if (vshCommandOptString(cmd, "snapshotname", &name) <= 0) goto cleanup; if (vshCommandOptBool(cmd, "children")) @@ -10270,7 +10193,7 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom = NULL; int ret = FALSE; - char *monitor_cmd; + char *monitor_cmd = NULL; char *result = NULL; unsigned int flags = 0; @@ -10281,8 +10204,7 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd) if (dom == NULL) goto cleanup; - monitor_cmd = vshCommandOptString(cmd, "cmd", NULL); - if (monitor_cmd == NULL) { + if (vshCommandOptString(cmd, "cmd", &monitor_cmd) <= 0) { vshError(ctl, "%s", _("missing monitor command")); goto cleanup; } @@ -10798,83 +10720,103 @@ vshCommandOpt(const vshCmd *cmd, const char *name) } /* - * Returns option as INT + * @cmd command reference + * @name option name + * @value result + * + * Convert option to int + * Return value: + * >0 if option found and valid (@value updated) + * 0 in case option not found (@value untouched) + * <0 in all other cases (@value untouched) */ static int -vshCommandOptInt(const vshCmd *cmd, const char *name, int *found) +vshCommandOptInt(const vshCmd *cmd, const char *name, int *value) { vshCmdOpt *arg = vshCommandOpt(cmd, name); - int res = 0, num_found = FALSE; + int ret = 0, num; char *end_p = NULL; if ((arg != NULL) && (arg->data != NULL)) { - res = strtol(arg->data, &end_p, 10); - if ((arg->data == end_p) || (*end_p!= 0)) - num_found = FALSE; - else - num_found = TRUE; + num = strtol(arg->data, &end_p, 10); + ret = -1; + if ((arg->data != end_p) && (*end_p == 0) && value) { + *value = num; + ret = 1; + } } - if (found) - *found = num_found; - return res; + return ret; } -static unsigned long -vshCommandOptUL(const vshCmd *cmd, const char *name, int *found) +/* + * Convert option to unsigned long + * See vshCommandOptInt() + */ +static int +vshCommandOptUL(const vshCmd *cmd, const char *name, unsigned long *value) { vshCmdOpt *arg = vshCommandOpt(cmd, name); - unsigned long res = 0; - int num_found = FALSE; + int ret = 0; + unsigned long num; char *end_p = NULL; if ((arg != NULL) && (arg->data != NULL)) { - res = strtoul(arg->data, &end_p, 10); - if ((arg->data == end_p) || (*end_p!= 0)) - num_found = FALSE; - else - num_found = TRUE; + num = strtoul(arg->data, &end_p, 10); + ret = -1; + if ((arg->data != end_p) && (*end_p == 0) && value) { + *value = num; + ret = 1; + } } - if (found) - *found = num_found; - return res; + return ret; } /* * Returns option as STRING + * See vshCommandOptInt() */ -static char * -vshCommandOptString(const vshCmd *cmd, const char *name, int *found) +static int +vshCommandOptString(const vshCmd *cmd, const char *name, char **value) { vshCmdOpt *arg = vshCommandOpt(cmd, name); + int ret = 0; + + if (arg && arg->data) { + ret = -1; + if (*arg->data) { + if (value) { + *value = arg->data; + ret = 1; + } + } else if (arg->def && ((arg->def->flag) & VSH_OFLAG_REQ)) { + vshError(NULL, _("Missing required option '%s'"), name); + } + } - if (found) - *found = arg ? TRUE : FALSE; - - if (arg && arg->data && *arg->data) - return arg->data; - - if (arg && arg->def && ((arg->def->flag) & VSH_OFLAG_REQ)) - vshError(NULL, _("Missing required option '%s'"), name); - - return NULL; + return ret; } /* * Returns option as long long */ -static long long -vshCommandOptLongLong(const vshCmd *cmd, const char *name, int *found) +static int +vshCommandOptLongLong(const vshCmd *cmd, const char *name, + unsigned long long *value) { vshCmdOpt *arg = vshCommandOpt(cmd, name); - int num_found = FALSE; - long long res = 0; + int ret = 0; + long long num; char *end_p = NULL; - if ((arg != NULL) && (arg->data != NULL)) - num_found = !virStrToLong_ll(arg->data, &end_p, 10, &res); - if (found) - *found = num_found; - return res; + if ((arg != NULL) && (arg->data != NULL)) { + num = strtoll(arg->data, &end_p, 10); + ret = -1; + if ((arg->data != end_p) && (*end_p == 0) && value) { + *value = num; + ret = 1; + } + } + return ret; } /* @@ -10935,13 +10877,13 @@ vshCommandOptDomainBy(vshControl *ctl, const vshCmd *cmd, char **name, int flag) { virDomainPtr dom = NULL; - char *n; + char *n = NULL; int id; const char *optname = "domain"; if (!cmd_has_option (ctl, cmd, optname)) return NULL; - if (!(n = vshCommandOptString(cmd, optname, NULL))) + if (vshCommandOptString(cmd, optname, &n) <= 0) return NULL; vshDebug(ctl, 5, "%s: found option <%s>: %s\n", @@ -10982,12 +10924,12 @@ vshCommandOptNetworkBy(vshControl *ctl, const vshCmd *cmd, char **name, int flag) { virNetworkPtr network = NULL; - char *n; + char *n = NULL; const char *optname = "network"; if (!cmd_has_option (ctl, cmd, optname)) return NULL; - if (!(n = vshCommandOptString(cmd, optname, NULL))) + if (vshCommandOptString(cmd, optname, &n) <= 0) return NULL; vshDebug(ctl, 5, "%s: found option <%s>: %s\n", @@ -11021,12 +10963,12 @@ vshCommandOptNWFilterBy(vshControl *ctl, const vshCmd *cmd, char **name, int flag) { virNWFilterPtr nwfilter = NULL; - char *n; + char *n = NULL; const char *optname = "nwfilter"; if (!cmd_has_option (ctl, cmd, optname)) return NULL; - if (!(n = vshCommandOptString(cmd, optname, NULL))) + if (vshCommandOptString(cmd, optname, &n) <= 0) return NULL; vshDebug(ctl, 5, "%s: found option <%s>: %s\n", @@ -11059,12 +11001,12 @@ vshCommandOptInterfaceBy(vshControl *ctl, const vshCmd *cmd, char **name, int flag) { virInterfacePtr iface = NULL; - char *n; + char *n = NULL; const char *optname = "interface"; if (!cmd_has_option (ctl, cmd, optname)) return NULL; - if (!(n = vshCommandOptString(cmd, optname, NULL))) + if (vshCommandOptString(cmd, optname, &n) <= 0) return NULL; vshDebug(ctl, 5, "%s: found option <%s>: %s\n", @@ -11097,9 +11039,9 @@ vshCommandOptPoolBy(vshControl *ctl, const vshCmd *cmd, const char *optname, char **name, int flag) { virStoragePoolPtr pool = NULL; - char *n; + char *n = NULL; - if (!(n = vshCommandOptString(cmd, optname, NULL))) + if (vshCommandOptString(cmd, optname, &n) <= 0) return NULL; vshDebug(ctl, 5, "%s: found option <%s>: %s\n", @@ -11135,13 +11077,12 @@ vshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd, { virStorageVolPtr vol = NULL; virStoragePoolPtr pool = NULL; - char *n, *p; - int found; + char *n = NULL, *p = NULL; - if (!(n = vshCommandOptString(cmd, optname, NULL))) + if (vshCommandOptString(cmd, optname, &n) <= 0) return NULL; - if (!(p = vshCommandOptString(cmd, pooloptname, &found)) && found) + if ( vshCommandOptString(cmd, pooloptname, &p) < 0) return NULL; if (p) @@ -11185,14 +11126,13 @@ static virSecretPtr vshCommandOptSecret(vshControl *ctl, const vshCmd *cmd, char **name) { virSecretPtr secret = NULL; - char *n; + char *n = NULL; const char *optname = "secret"; if (!cmd_has_option (ctl, cmd, optname)) return NULL; - n = vshCommandOptString(cmd, optname, NULL); - if (n == NULL) + if (vshCommandOptString(cmd, optname, &n) <= 0) return NULL; vshDebug(ctl, 5, "%s: found option <%s>: %s\n", cmd->def->name, optname, n); -- 1.7.4

2 1

[libvirt] [PATCH 1/2] rename qemuDomainDetachSCSIDiskDevice to qemuDomainDetachDiskDevice
by Wen Congyang 07 Mar '11

07 Mar '11

The way to detach a USB disk is the same as that to detach a SCSI disk. Rename this function and we can use it to detach a USB disk. Signed-off-by: Wen Congyang <wency(a)cn.fujitsu.com> --- src/qemu/qemu_driver.c | 3 +-- src/qemu/qemu_hotplug.c | 13 +++++++------ src/qemu/qemu_hotplug.h | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0e891e8..f4e52b1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4281,8 +4281,7 @@ static int qemudDomainDetachDevice(virDomainPtr dom, ret = qemuDomainDetachPciDiskDevice(driver, vm, dev, qemuCaps); } else if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) { - ret = qemuDomainDetachSCSIDiskDevice(driver, vm, dev, - qemuCaps); + ret = qemuDomainDetachDiskDevice(driver, vm, dev, qemuCaps); } else { qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index da07c29..0b822bf 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1221,10 +1221,10 @@ cleanup: return ret; } -int qemuDomainDetachSCSIDiskDevice(struct qemud_driver *driver, - virDomainObjPtr vm, - virDomainDeviceDefPtr dev, - virBitmapPtr qemuCaps) +int qemuDomainDetachDiskDevice(struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + virBitmapPtr qemuCaps) { int i, ret = -1; virDomainDiskDefPtr detach = NULL; @@ -1241,8 +1241,9 @@ int qemuDomainDetachSCSIDiskDevice(struct qemud_driver *driver, } if (!qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("Underlying qemu does not support SCSI disk removal")); + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("Underlying qemu does not support %s disk removal"), + virDomainDiskBusTypeToString(dev->data.disk->bus)); goto cleanup; } diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h index c84d496..db3970c 100644 --- a/src/qemu/qemu_hotplug.h +++ b/src/qemu/qemu_hotplug.h @@ -77,10 +77,10 @@ int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver, virDomainObjPtr vm, virDomainDeviceDefPtr dev, virBitmapPtr qemuCaps); -int qemuDomainDetachSCSIDiskDevice(struct qemud_driver *driver, - virDomainObjPtr vm, - virDomainDeviceDefPtr dev, - virBitmapPtr qemuCaps); +int qemuDomainDetachDiskDevice(struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + virBitmapPtr qemuCaps); int qemuDomainDetachPciControllerDevice(struct qemud_driver *driver, virDomainObjPtr vm, virDomainDeviceDefPtr dev, -- 1.7.1

3 4

[libvirt] [PATCH] qemu_hotplug: Reword error if spice password change not available
by Cole Robinson 07 Mar '11

07 Mar '11

Currently it sounds like spice is completely unsupported, which is confusing. --- src/qemu/qemu_hotplug.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index da07c29..25d2dd9 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1795,7 +1795,7 @@ qemuDomainChangeGraphicsPasswords(struct qemud_driver *driver, if (ret == -2) { if (type != VIR_DOMAIN_GRAPHICS_TYPE_VNC) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Only VNC graphics are supported")); + _("Graphics password only supported for VNC")); ret = -1; } else { ret = qemuMonitorSetVNCPassword(priv->mon, -- 1.7.4

2 2

[libvirt] [PATCH] unlock eventLoop before calling callback function
by Wen Congyang 07 Mar '11

07 Mar '11

When I use newest libvirt to save a domain, libvirtd will be deadlock. Here is the output of gdb: (gdb) thread 3 [Switching to thread 3 (Thread 0x7f972a1fc710 (LWP 30265))]#0 0x000000351fe0e034 in __lll_lock_wait () from /lib64/libpthread.so.0 (gdb) bt #0 0x000000351fe0e034 in __lll_lock_wait () from /lib64/libpthread.so.0 #1 0x000000351fe09345 in _L_lock_870 () from /lib64/libpthread.so.0 #2 0x000000351fe09217 in pthread_mutex_lock () from /lib64/libpthread.so.0 #3 0x00007f9737b0e663 in virMutexLock (m=0x6fdd60) at util/threads-pthread.c:80 #4 0x000000000041853d in virEventUpdateTimeoutImpl (timer=12, frequency=0) at event.c:247 #5 0x00007f9737af8ac2 in virEventUpdateTimeout (timer=12, timeout=0) at util/event.c:70 #6 0x000000000046654e in qemuDomainEventQueue (driver=0x19c5440, event=0x7f9724005f80) at qemu/qemu_domain.c:97 #7 0x000000000044184a in qemudDomainSaveFlag (driver=0x19c5440, dom=0x7f9724000970, vm=0x1a63ac0, path=0x7f9724000940 "/var/lib/libvirt/images/memory.save", compressed=0) at qemu/qemu_driver.c:2074 #8 0x0000000000441b30 in qemudDomainSave (dom=0x7f9724000970, path=0x7f9724000940 "/var/lib/libvirt/images/memory.save") at qemu/qemu_driver.c:2137 #9 0x00007f9737b66d7d in virDomainSave (domain=0x7f9724000970, to=0x7f9724000940 "/var/lib/libvirt/images/memory.save") at libvirt.c:2280 #10 0x00000000004273e6 in remoteDispatchDomainSave (server=0x19ac120, client=0x7f972c081740, conn=0x7f9720000b40, hdr=0x7f972c041250, rerr=0x7f972a1fbb40, args=0x7f972a1fbc40, ret=0x7f972a1fbbe0) at remote.c:2273 #11 0x0000000000431b6f in remoteDispatchClientCall (server=0x19ac120, client=0x7f972c081740, msg=0x7f972c001240, qemu_protocol=false) at dispatch.c:529 #12 0x00000000004316fe in remoteDispatchClientRequest (server=0x19ac120, client=0x7f972c081740, msg=0x7f972c001240) at dispatch.c:407 #13 0x000000000041d9a2 in qemudWorker (data=0x7f972c000908) at libvirtd.c:1629 #14 0x000000351fe077e1 in start_thread () from /lib64/libpthread.so.0 #15 0x000000351f2e153d in clone () from /lib64/libc.so.6 (gdb) thread 7 [Switching to thread 7 (Thread 0x7f9730bcd710 (LWP 30261))]#0 0x000000351fe0e034 in __lll_lock_wait () from /lib64/libpthread.so.0 (gdb) bt #0 0x000000351fe0e034 in __lll_lock_wait () from /lib64/libpthread.so.0 #1 0x000000351fe09345 in _L_lock_870 () from /lib64/libpthread.so.0 #2 0x000000351fe09217 in pthread_mutex_lock () from /lib64/libpthread.so.0 #3 0x00007f9737b0e663 in virMutexLock (m=0x1a63ac0) at util/threads-pthread.c:80 #4 0x00007f9737b32cb8 in virDomainObjLock (obj=0x1a63ac0) at conf/domain_conf.c:8447 #5 0x00000000004732d2 in qemuProcessHandleMonitorDestroy (mon=0x7f9718002610, vm=0x1a63ac0) at qemu/qemu_process.c:599 #6 0x000000000047c05b in qemuMonitorFree (mon=0x7f9718002610) at qemu/qemu_monitor.c:209 #7 0x000000000047c0f9 in qemuMonitorUnref (mon=0x7f9718002610) at qemu/qemu_monitor.c:229 #8 0x000000000047c135 in qemuMonitorUnwatch (monitor=0x7f9718002610) at qemu/qemu_monitor.c:242 #9 0x00000000004194b4 in virEventCleanupHandles () at event.c:538 #10 0x00000000004197a4 in virEventRunOnce () at event.c:603 #11 0x000000000041f0bd in qemudOneLoop () at libvirtd.c:2285 #12 0x000000000041f5e9 in qemudRunLoop (opaque=0x19ac120) at libvirtd.c:2395 #13 0x000000351fe077e1 in start_thread () from /lib64/libpthread.so.0 #14 0x000000351f2e153d in clone () from /lib64/libc.so.6 (gdb) p *(virMutexPtr)0x6fdd60 $2 = {lock = {__data = {__lock = 2, __count = 0, __owner = 30261, __nusers = 1, __kind = 0, __spins = 0, __list = {__prev = 0x0, __next = 0x0}}, __size = "\002\000\000\000\000\000\000\000\065v\000\000\001", '\000' <repeats 26 times>, __align = 2}} (gdb) p *(virMutexPtr)0x1a63ac0 $3 = {lock = {__data = {__lock = 2, __count = 0, __owner = 30265, __nusers = 1, __kind = 0, __spins = 0, __list = {__prev = 0x0, __next = 0x0}}, __size = "\002\000\000\000\000\000\000\000\071v\000\000\001", '\000' <repeats 26 times>, __align = 2}} (gdb) info threads 7 Thread 0x7f9730bcd710 (LWP 30261) 0x000000351fe0e034 in __lll_lock_wait () from /lib64/libpthread.so.0 6 Thread 0x7f972bfff710 (LWP 30262) 0x000000351fe0b43c in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0 5 Thread 0x7f972b5fe710 (LWP 30263) 0x000000351fe0b43c in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0 4 Thread 0x7f972abfd710 (LWP 30264) 0x000000351fe0b43c in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0 * 3 Thread 0x7f972a1fc710 (LWP 30265) 0x000000351fe0e034 in __lll_lock_wait () from /lib64/libpthread.so.0 2 Thread 0x7f97297fb710 (LWP 30266) 0x000000351fe0b43c in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0 1 Thread 0x7f9737aac800 (LWP 30260) 0x000000351fe0803d in pthread_join () from /lib64/libpthread.so.0 The reason is that we will try to lock some object in callback function, and we may call event API with locking the same object. In the function virEventDispatchHandles(), we unlock eventLoop before calling callback function. I think we should do the same thing in the function virEventCleanupTimeouts() and virEventCleanupHandles(). Signed-off-by: Wen Congyang <wency(a)cn.fujitsu.com> --- daemon/event.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-) diff --git a/daemon/event.c b/daemon/event.c index 1a31717..0d45014 100644 --- a/daemon/event.c +++ b/daemon/event.c @@ -493,8 +493,11 @@ static int virEventCleanupTimeouts(void) { EVENT_DEBUG("Purging timeout %d with id %d", i, eventLoop.timeouts[i].timer); - if (eventLoop.timeouts[i].ff) + if (eventLoop.timeouts[i].ff) { + virMutexUnlock(&eventLoop.lock); (eventLoop.timeouts[i].ff)(eventLoop.timeouts[i].opaque); + virMutexLock(&eventLoop.lock); + } if ((i+1) < eventLoop.timeoutsCount) { memmove(eventLoop.timeouts+i, @@ -534,8 +537,11 @@ static int virEventCleanupHandles(void) { continue; } - if (eventLoop.handles[i].ff) + if (eventLoop.handles[i].ff) { + virMutexUnlock(&eventLoop.lock); (eventLoop.handles[i].ff)(eventLoop.handles[i].opaque); + virMutexLock(&eventLoop.lock); + } if ((i+1) < eventLoop.handlesCount) { memmove(eventLoop.handles+i, -- 1.7.1

5 10