- Devel - libvirt List Archives

[libvirt] [PATCH] qemu: plug memory leak
by Eric Blake 15 Dec '10

15 Dec '10

https://bugzilla.redhat.com/show_bug.cgi?id=656795 * src/qemu/qemu_monitor.c (qemuMonitorFree): Also free the buffer. --- src/qemu/qemu_monitor.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 86ed82f..85d0d0f 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -199,6 +199,7 @@ static void qemuMonitorFree(qemuMonitorPtr mon) if (virCondDestroy(&mon->notify) < 0) {} virMutexDestroy(&mon->lock); + VIR_FREE(mon->buffer); VIR_FREE(mon); } -- 1.7.3.2

2 22

[libvirt] [PATCH 1/2] build: properly handle ./configure --with-libpcap
by Diego Elio Pettenò 15 Dec '10

15 Dec '10

Without this fix, ./configure --with-libpcap will cause --with-libpcap=yes to be implicitly passed down, which cause yes/bin/pcap-config to be searched for rather than /usr/bin/pcap-config. Also output pcap: no when pcap is not found or disabled. --- configure.ac | 30 ++++++++++++++++-------------- 1 files changed, 16 insertions(+), 14 deletions(-) diff --git a/configure.ac b/configure.ac index d8be160..b5c995f 100644 --- a/configure.ac +++ b/configure.ac @@ -1194,20 +1194,22 @@ LIBPCAP_FOUND="no" AC_ARG_WITH([libpcap], AC_HELP_STRING([--with-libpcap=@<:@PFX@:>@], [libpcap location])) if test "$with_qemu" = "yes"; then - if test "x$with_libpcap" != "xno" ; then - if test "x$with_libpcap" != "x" ; then - LIBPCAP_CONFIG=$with_libpcap/bin/$LIBPCAP_CONFIG - fi - AC_MSG_CHECKING(libpcap $LIBPCAP_CONFIG >= $LIBPCAP_REQUIRED ) - if ! $LIBPCAP_CONFIG --libs > /dev/null 2>&1 ; then - AC_MSG_RESULT(no) - else - LIBPCAP_LIBS="`$LIBPCAP_CONFIG --libs`" - LIBPCAP_CFLAGS="`$LIBPCAP_CONFIG --cflags`" - LIBPCAP_FOUND="yes" - AC_MSG_RESULT(yes) - fi + AS_CASE(["x$with_libpcap"], + [xno], [LIBPCAP_CONFIG=""], + [x|xyes], [LIBPCAP_CONFIG="pcap-config"], + [LIBPCAP_CONFIG="$with_libpcap/bin/pcap-config"] + ) + AS_IF([test "x$LIBPCAP_CONFIG" != "x"], [ + AC_MSG_CHECKING(libpcap $LIBPCAP_CONFIG >= $LIBPCAP_REQUIRED ) + if ! $LIBPCAP_CONFIG --libs > /dev/null 2>&1 ; then + AC_MSG_RESULT(no) + else + LIBPCAP_LIBS="`$LIBPCAP_CONFIG --libs`" + LIBPCAP_CFLAGS="`$LIBPCAP_CONFIG --cflags`" + LIBPCAP_FOUND="yes" + AC_MSG_RESULT(yes) fi + ]) fi if test "x$LIBPCAP_FOUND" = "xyes"; then @@ -2406,7 +2408,7 @@ AC_MSG_NOTICE([ xmlrpc: $XMLRPC_CFLAGS $XMLRPC_LIBS]) else AC_MSG_NOTICE([ xmlrpc: no]) fi -if test "$with_qemu" = "yes" ; then +if test "$with_qemu" = "yes" -a "$LIBPCAP_FOUND" != "no"; then AC_MSG_NOTICE([ pcap: $LIBPCAP_CFLAGS $LIBPCAP_LIBS]) else AC_MSG_NOTICE([ pcap: no]) -- 1.7.3.3

2 6

[libvirt] libvirt-guests output
by Laurent Léonard 14 Dec '10

14 Dec '10

Hi, Here is an example of libvirt-guest output: $Running guests on default URI: test-vm $Suspending guests on default URI... $Suspending test-vm: $done Why all the lines begin with "$" ? Thank you, -- Laurent Léonard

2 5

[libvirt] [PATCH 0/3] Makes the code more compact
by Hu Tao 14 Dec '10

14 Dec '10

These three patches introduce two new functions and one macro respectively, which make the code clean and compact. Hu Tao (3): Add a macro timeval_to_ms to compute micro seconds from timeval Add a new function doStopCPUs Add a new function doStartCPUs src/qemu/qemu_driver.c | 151 ++++++++++++++++++------------------------------ 1 files changed, 57 insertions(+), 94 deletions(-) -- 1.7.3 -- Thanks, Hu Tao

2 6

[libvirt] [libvirt PATCH] 802.1Qbh: Add support for IFLA_VF_MAC
by Roopa Prabhu 14 Dec '10

14 Dec '10

From: Roopa Prabhu <roprabhu(a)cisco.com> Current code does not pass VM mac address to a 802.1Qbh direct attach interface using IFLA_VF_MAC. This patch adds support in macvtap code to send IFLA_VF_MAC netlink request during port profile association on a 802.1Qbh interface. Stefan Cc'ed for comments because this patch changes a condition for 802.1Qbg 802.1Qbh support for IFLA_VF_MAC in enic driver has been posted and is pending acceptance at http://marc.info/?l=linux-netdev&m=129185244410557&w=2 Signed-off-by: Roopa Prabhu <roprabhu(a)cisco.com> Signed-off-by: David Wang <dwang2(a)cisco.com> Signed-off-by: Christian Benvenuti <benve(a)cisco.com> --- src/util/macvtap.c | 59 ++++++++++++++++++++++++++++------------------------ 1 files changed, 32 insertions(+), 27 deletions(-) diff --git a/src/util/macvtap.c b/src/util/macvtap.c index 4345d97..96df301 100644 --- a/src/util/macvtap.c +++ b/src/util/macvtap.c @@ -1031,19 +1031,8 @@ doPortProfileOpSetLink(bool nltarget_kernel, nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0) goto buffer_too_small; - if (macaddr && vlanid >= 0) { + if (macaddr || vlanid >= 0) { struct nlattr *vfinfolist, *vfinfo; - struct ifla_vf_mac ifla_vf_mac = { - .vf = vf, - .mac = { 0, }, - }; - struct ifla_vf_vlan ifla_vf_vlan = { - .vf = vf, - .vlan = vlanid, - .qos = 0, - }; - - memcpy(ifla_vf_mac.mac, macaddr, 6); if (!(vfinfolist = nla_nest_start(nl_msg, IFLA_VFINFO_LIST))) goto buffer_too_small; @@ -1051,13 +1040,30 @@ doPortProfileOpSetLink(bool nltarget_kernel, if (!(vfinfo = nla_nest_start(nl_msg, IFLA_VF_INFO))) goto buffer_too_small; - if (!nla_put(nl_msg, IFLA_VF_MAC, sizeof(ifla_vf_mac), - &ifla_vf_mac) < 0) - goto buffer_too_small; + if (macaddr) { + struct ifla_vf_mac ifla_vf_mac = { + .vf = vf, + .mac = { 0, }, + }; - if (!nla_put(nl_msg, IFLA_VF_VLAN, sizeof(ifla_vf_vlan), - &ifla_vf_vlan) < 0) - goto buffer_too_small; + memcpy(ifla_vf_mac.mac, macaddr, 6); + + if (!nla_put(nl_msg, IFLA_VF_MAC, sizeof(ifla_vf_mac), + &ifla_vf_mac) < 0) + goto buffer_too_small; + } + + if (vlanid >= 0) { + struct ifla_vf_vlan ifla_vf_vlan = { + .vf = vf, + .vlan = vlanid, + .qos = 0, + }; + + if (!nla_put(nl_msg, IFLA_VF_VLAN, sizeof(ifla_vf_vlan), + &ifla_vf_vlan) < 0) + goto buffer_too_small; + } nla_nest_end(nl_msg, vfinfo); nla_nest_end(nl_msg, vfinfolist); @@ -1402,6 +1408,7 @@ getPhysfn(const char *linkdev, static int doPortProfileOp8021Qbh(const char *ifname, + const unsigned char *macaddr, const virVirtualPortProfileParamsPtr virtPort, const unsigned char *vm_uuid, enum virVirtualPortOp virtPortOp) @@ -1411,6 +1418,7 @@ doPortProfileOp8021Qbh(const char *ifname, # ifndef IFLA_VF_PORT_MAX (void)ifname; + (void)macaddr; (void)virtPort; (void)vm_uuid; (void)virtPortOp; @@ -1426,7 +1434,6 @@ doPortProfileOp8021Qbh(const char *ifname, bool nltarget_kernel = true; int ifindex; int vlanid = -1; - const unsigned char *macaddr = NULL; rc = getPhysfn(ifname, &vf, &physfndev); if (rc) @@ -1456,7 +1463,7 @@ doPortProfileOp8021Qbh(const char *ifname, /* Association timed out, disassociate */ doPortProfileOpCommon(nltarget_kernel, NULL, ifindex, NULL, - 0, + vlanid, NULL, NULL, NULL, @@ -1470,7 +1477,7 @@ doPortProfileOp8021Qbh(const char *ifname, case DISASSOCIATE: rc = doPortProfileOpCommon(nltarget_kernel, NULL, ifindex, NULL, - 0, + vlanid, NULL, NULL, NULL, @@ -1545,9 +1552,8 @@ vpAssociatePortProfileId(const char *macvtap_ifname, /* avoid associating twice */ if (vmOp == VIR_VM_OP_MIGRATE_IN_FINISH) break; - rc = doPortProfileOp8021Qbh(linkdev, virtPort, - vmuuid, - ASSOCIATE); + rc = doPortProfileOp8021Qbh(linkdev, macvtap_macaddr, + virtPort, vmuuid, ASSOCIATE); break; } @@ -1594,9 +1600,8 @@ vpDisassociatePortProfileId(const char *macvtap_ifname, /* avoid disassociating twice */ if (vmOp == VIR_VM_OP_MIGRATE_IN_FINISH) break; - rc = doPortProfileOp8021Qbh(linkdev, virtPort, - NULL, - DISASSOCIATE); + rc = doPortProfileOp8021Qbh(linkdev, macvtap_macaddr, + virtPort, NULL, DISASSOCIATE); break; }

3 3

[libvirt] [PATCH] daemon: plug a memory leak
by Eric Blake 14 Dec '10

14 Dec '10

* daemon/libvirtd.c (qemudStartWorker, qemudStartEventLoop): Avoid leaking pthread_attr resources. --- daemon/libvirtd.c | 36 ++++++++++++++++++++++++++---------- 1 files changed, 26 insertions(+), 10 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 2914f2f..14777dd 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1600,15 +1600,20 @@ static void *qemudWorker(void *data) } } -static int qemudStartWorker(struct qemud_server *server, - struct qemud_worker *worker) { +static int +qemudStartWorker(struct qemud_server *server, + struct qemud_worker *worker) +{ pthread_attr_t attr; - pthread_attr_init(&attr); + int ret = -1; + + if (pthread_attr_init(&attr) != 0) + return -1; /* We want to join workers, so don't detach them */ /*pthread_attr_setdetachstate(&attr, 1);*/ if (worker->hasThread) - return -1; + goto cleanup; worker->server = server; worker->hasThread = 1; @@ -1621,10 +1626,13 @@ static int qemudStartWorker(struct qemud_server *server, worker) != 0) { worker->hasThread = 0; worker->server = NULL; - return -1; + goto cleanup; } - return 0; + ret = 0; +cleanup: + pthread_attr_destroy(&attr); + return ret; } @@ -2414,9 +2422,14 @@ cleanup: } -static int qemudStartEventLoop(struct qemud_server *server) { +static int +qemudStartEventLoop(struct qemud_server *server) +{ pthread_attr_t attr; - pthread_attr_init(&attr); + int ret = -1; + + if (pthread_attr_init(&attr) != 0) + return -1; /* We want to join the eventloop, so don't detach it */ /*pthread_attr_setdetachstate(&attr, 1);*/ @@ -2424,11 +2437,14 @@ static int qemudStartEventLoop(struct qemud_server *server) { &attr, qemudRunLoop, server) != 0) - return -1; + goto cleanup; server->hasEventThread = 1; - return 0; + ret = 0; +cleanup: + pthread_attr_destroy(&attr); + return ret; } -- 1.7.3.3

3 3

[libvirt] [PATCH] util: Fix logical error in virReportSystemErrorFull
by Jiri Denemark 14 Dec '10

14 Dec '10

--- src/util/virterror.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/util/virterror.c b/src/util/virterror.c index 6e49fa2..b912737 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -1301,7 +1301,7 @@ void virReportSystemErrorFull(int domcode, n = vsnprintf(msgDetailBuf, sizeof(msgDetailBuf), fmt, args); va_end(args); - size_t len = strlen (msgDetailBuf); + size_t len = strlen(errnoDetail); if (0 <= n && n + 2 + len < sizeof (msgDetailBuf)) { char *p = msgDetailBuf + n; stpcpy (stpcpy (p, ": "), errnoDetail); -- 1.7.3.3

2 2

[libvirt] [PATCH] util: Fix error message in __virExec
by Jiri Denemark 14 Dec '10

14 Dec '10

Remove superfluous ": %s" suffix from the error message. --- src/util/util.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 1b5bc68..e24facb 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -617,7 +617,7 @@ __virExec(const char *const*argv, if (chdir("/") < 0) { virReportSystemError(errno, - "%s", _("cannot change to root directory: %s")); + "%s", _("cannot change to root directory")); goto fork_error; } -- 1.7.3.3

2 2

[libvirt] [PATCH] daemon: Change CWD to / before daemonizing
by Jiri Denemark 14 Dec '10

14 Dec '10

We were doing so for child processes but not for libvirtd itself. --- daemon/libvirtd.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 791b3dc..58fc492 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -3158,6 +3158,13 @@ int main(int argc, char **argv) { if (godaemon) { char ebuf[1024]; + + if (chdir("/") < 0) { + VIR_ERROR(_("cannot change to root directory: %s"), + virStrerror(errno, ebuf, sizeof(ebuf))); + goto error; + } + if ((statuswrite = daemonForkIntoBackground()) < 0) { VIR_ERROR(_("Failed to fork as daemon: %s"), virStrerror(errno, ebuf, sizeof ebuf)); -- 1.7.3.3

2 2

[libvirt] [PATCH] cpu: Unify CPUID data structures
by Jiri Denemark 14 Dec '10

14 Dec '10

So far, CPUID data were stored in two different data structures. First of them was a structure allowing direct access for CPUID data according to function number and the second was a plain array of struct cpuX86cpuid. This was a silly design which resulted in converting data from one type to the other and back again or implementing similar functionality for both data structures. The patch leaves only the direct access structure. This makes the code both smaller and more maintainable since operations on different objects can use common low-level operations. All 57 tests for cpu subsystem still pass after this rewrite. --- src/cpu/cpu_x86.c | 576 +++++++++++++++++++++-------------------------- src/cpu/cpu_x86_data.h | 4 +- 2 files changed, 259 insertions(+), 321 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 425a8eb..df1e431 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -37,6 +37,7 @@ #define VENDOR_STRING_LENGTH 12 +static const struct cpuX86cpuid cpuidNull = { 0, 0, 0, 0, 0 }; static const char *archs[] = { "i686", "x86_64" }; @@ -49,8 +50,7 @@ struct x86_vendor { struct x86_feature { char *name; - unsigned int ncpuid; - struct cpuX86cpuid *cpuid; + union cpuData *data; struct x86_feature *next; }; @@ -58,8 +58,7 @@ struct x86_feature { struct x86_model { char *name; const struct x86_vendor *vendor; - unsigned int ncpuid; - struct cpuX86cpuid *cpuid; + union cpuData *data; struct x86_model *next; }; @@ -79,23 +78,28 @@ enum compare_result { }; -static struct cpuX86cpuid * -x86cpuidFind(struct cpuX86cpuid *cpuids, - unsigned int ncpuids, - uint32_t function) -{ - unsigned int i; +struct data_iterator { + union cpuData *data; + int pos; + bool extended; +}; - for (i = 0; i < ncpuids; i++) { - if (cpuids[i].function == function) - return cpuids + i; - } - return NULL; +#define DATA_ITERATOR_INIT(data) \ + { data, -1, false } + + +static void +x86DataIteratorInit(struct data_iterator *iter, + union cpuData *data) +{ + struct data_iterator init = DATA_ITERATOR_INIT(data); + + *iter = init; } -static inline int +static int x86cpuidMatch(const struct cpuX86cpuid *cpuid1, const struct cpuX86cpuid *cpuid2) { @@ -106,7 +110,7 @@ x86cpuidMatch(const struct cpuX86cpuid *cpuid1, } -static inline int +static int x86cpuidMatchMasked(const struct cpuX86cpuid *cpuid, const struct cpuX86cpuid *mask) { @@ -117,7 +121,7 @@ x86cpuidMatchMasked(const struct cpuX86cpuid *cpuid, } -static inline int +static int x86cpuidMatchAny(const struct cpuX86cpuid *cpuid, const struct cpuX86cpuid *mask) { @@ -128,7 +132,7 @@ x86cpuidMatchAny(const struct cpuX86cpuid *cpuid, } -static inline void +static void x86cpuidSetBits(struct cpuX86cpuid *cpuid, const struct cpuX86cpuid *mask) { @@ -139,7 +143,7 @@ x86cpuidSetBits(struct cpuX86cpuid *cpuid, } -static inline void +static void x86cpuidClearBits(struct cpuX86cpuid *cpuid, const struct cpuX86cpuid *mask) { @@ -150,7 +154,7 @@ x86cpuidClearBits(struct cpuX86cpuid *cpuid, } -static inline void +static void x86cpuidAndBits(struct cpuX86cpuid *cpuid, const struct cpuX86cpuid *mask) { @@ -161,6 +165,40 @@ x86cpuidAndBits(struct cpuX86cpuid *cpuid, } +/* skips all zero CPUID leafs */ +static struct cpuX86cpuid * +x86DataCpuidNext(struct data_iterator *iterator) +{ + struct cpuX86cpuid *ret; + struct cpuX86Data *data; + + if (!iterator->data) + return NULL; + + data = &iterator->data->x86; + + do { + ret = NULL; + iterator->pos++; + + if (!iterator->extended) { + if (iterator->pos < data->basic_len) + ret = data->basic + iterator->pos; + else { + iterator->extended = true; + iterator->pos = 0; + } + } + + if (iterator->extended && iterator->pos < data->extended_len) { + ret = data->extended + iterator->pos; + } + } while (ret && x86cpuidMatch(ret, &cpuidNull)); + + return ret; +} + + static struct cpuX86cpuid * x86DataCpuid(const union cpuData *data, uint32_t function) @@ -180,7 +218,7 @@ x86DataCpuid(const union cpuData *data, i = function - CPUX86_EXTENDED; } - if (i < len) + if (i < len && !x86cpuidMatch(cpuids + i, &cpuidNull)) return cpuids + i; else return NULL; @@ -225,38 +263,86 @@ x86DataCopy(const union cpuData *data) static int +x86DataExpand(union cpuData *data, + int basic_by, + int extended_by) +{ + size_t i; + + if (basic_by > 0) { + size_t len = data->x86.basic_len; + if (VIR_EXPAND_N(data->x86.basic, data->x86.basic_len, basic_by) < 0) + goto no_memory; + + for (i = 0; i < basic_by; i++) + data->x86.basic[len + i].function = len + i; + } + + if (extended_by > 0) { + size_t len = data->x86.extended_len; + if (VIR_EXPAND_N(data->x86.extended, data->x86.extended_len, extended_by) < 0) + goto no_memory; + + for (i = 0; i < extended_by; i++) + data->x86.extended[len + i].function = len + i + CPUX86_EXTENDED; + } + + return 0; + +no_memory: + virReportOOMError(); + return -1; +} + + +static int x86DataAddCpuid(union cpuData *data, const struct cpuX86cpuid *cpuid) { + unsigned int basic_by = 0; + unsigned int extended_by = 0; struct cpuX86cpuid **cpuids; - int *len; unsigned int pos; - unsigned int ext; if (cpuid->function < CPUX86_EXTENDED) { pos = cpuid->function; - ext = 0; - len = &data->x86.basic_len; + basic_by = pos + 1 - data->x86.basic_len; cpuids = &data->x86.basic; } else { pos = cpuid->function - CPUX86_EXTENDED; - ext = CPUX86_EXTENDED; - len = &data->x86.extended_len; + extended_by = pos + 1 - data->x86.extended_len; cpuids = &data->x86.extended; } - if (pos >= *len) { - unsigned int i; + if (x86DataExpand(data, basic_by, extended_by) < 0) + return -1; + + x86cpuidSetBits((*cpuids) + pos, cpuid); - if (VIR_ALLOC_N(*cpuids, pos + 1) < 0) - return -1; + return 0; +} + + +static int +x86DataAdd(union cpuData *data1, + const union cpuData *data2) +{ + unsigned int i; + + if (x86DataExpand(data1, + data2->x86.basic_len - data1->x86.basic_len, + data2->x86.extended_len - data1->x86.extended_len) < 0) + return -1; - for (i = *len; i <= pos; i++) - (*cpuids)[i].function = i + ext; - *len = pos + 1; + for (i = 0; i < data2->x86.basic_len; i++) { + x86cpuidSetBits(data1->x86.basic + i, + data2->x86.basic + i); } - x86cpuidSetBits((*cpuids) + pos, cpuid); + for (i = 0; i < data2->x86.extended_len; i++) { + x86cpuidSetBits(data1->x86.extended + i, + data2->x86.extended + i); + } return 0; } @@ -283,61 +369,49 @@ x86DataSubtract(union cpuData *data1, } -static bool -x86DataIsEmpty(union cpuData *data) +static void +x86DataIntersect(union cpuData *data1, + const union cpuData *data2) { - struct cpuX86cpuid zero = { 0, 0, 0, 0, 0 }; - unsigned int i; - - for (i = 0; i < data->x86.basic_len; i++) { - if (!x86cpuidMatch(data->x86.basic + i, &zero)) - return false; - } + struct data_iterator iter = DATA_ITERATOR_INIT(data1); + struct cpuX86cpuid *cpuid1; + struct cpuX86cpuid *cpuid2; - for (i = 0; i < data->x86.extended_len; i++) { - if (!x86cpuidMatch(data->x86.extended + i, &zero)) - return false; + while ((cpuid1 = x86DataCpuidNext(&iter))) { + cpuid2 = x86DataCpuid(data2, cpuid1->function); + if (cpuid2) + x86cpuidAndBits(cpuid1, cpuid2); + else + x86cpuidClearBits(cpuid1, cpuid1); } - - return true; } -static union cpuData * -x86DataFromModel(const struct x86_model *model) +static bool +x86DataIsEmpty(union cpuData *data) { - union cpuData *data = NULL; - uint32_t basic_len = 0; - uint32_t extended_len = 0; - struct cpuX86cpuid *cpuid; - int i; + struct data_iterator iter = DATA_ITERATOR_INIT(data); - for (i = 0; i < model->ncpuid; i++) { - cpuid = model->cpuid + i; - if (cpuid->function < CPUX86_EXTENDED) { - if (cpuid->function >= basic_len) - basic_len = cpuid->function + 1; - } - else if (cpuid->function - CPUX86_EXTENDED >= extended_len) - extended_len = cpuid->function - CPUX86_EXTENDED + 1; - } + return (x86DataCpuidNext(&iter) == NULL); +} - if (VIR_ALLOC(data) < 0 - || VIR_ALLOC_N(data->x86.basic, basic_len) < 0 - || VIR_ALLOC_N(data->x86.extended, extended_len) < 0) { - x86DataFree(data); - return NULL; - } - data->x86.basic_len = basic_len; - data->x86.extended_len = extended_len; +static bool +x86DataIsSubset(const union cpuData *data, + const union cpuData *subset) +{ + + struct data_iterator iter = DATA_ITERATOR_INIT((union cpuData *) subset); + const struct cpuX86cpuid *cpuid; + const struct cpuX86cpuid *cpuidSubset; - for (i = 0; i < model->ncpuid; i++) { - cpuid = x86DataCpuid(data, model->cpuid[i].function); - *cpuid = model->cpuid[i]; + while ((cpuidSubset = x86DataCpuidNext(&iter))) { + if (!(cpuid = x86DataCpuid(data, cpuidSubset->function)) || + !x86cpuidMatchMasked(cpuid, cpuidSubset)) + return false; } - return data; + return true; } @@ -349,17 +423,12 @@ x86DataToCPUFeatures(virCPUDefPtr cpu, const struct x86_map *map) { const struct x86_feature *feature = map->features; - struct cpuX86cpuid *cpuid; - unsigned int i; while (feature != NULL) { - for (i = 0; i < feature->ncpuid; i++) { - if ((cpuid = x86DataCpuid(data, feature->cpuid[i].function)) - && x86cpuidMatchMasked(cpuid, feature->cpuid + i)) { - x86cpuidClearBits(cpuid, feature->cpuid + i); - if (virCPUDefAddFeature(cpu, feature->name, policy) < 0) - return -1; - } + if (x86DataIsSubset(data, feature->data)) { + x86DataSubtract(data, feature->data); + if (virCPUDefAddFeature(cpu, feature->name, policy) < 0) + return -1; } feature = feature->next; } @@ -402,7 +471,7 @@ x86DataToCPU(const union cpuData *data, if (VIR_ALLOC(cpu) < 0 || !(cpu->model = strdup(model->name)) || !(copy = x86DataCopy(data)) || - !(modelData = x86DataFromModel(model))) + !(modelData = x86DataCopy(model->data))) goto no_memory; if ((vendor = x86DataToVendor(copy, map)) && @@ -441,7 +510,7 @@ x86VendorFree(struct x86_vendor *vendor) VIR_FREE(vendor->name); VIR_FREE(vendor); -}; +} static struct x86_vendor * @@ -533,6 +602,23 @@ ignore: } +static struct x86_feature * +x86FeatureNew(void) +{ + struct x86_feature *feature; + + if (VIR_ALLOC(feature) < 0) + return NULL; + + if (VIR_ALLOC(feature->data) < 0) { + VIR_FREE(feature); + return NULL; + } + + return feature; +} + + static void x86FeatureFree(struct x86_feature *feature) { @@ -540,7 +626,7 @@ x86FeatureFree(struct x86_feature *feature) return; VIR_FREE(feature->name); - VIR_FREE(feature->cpuid); + x86DataFree(feature->data); VIR_FREE(feature); } @@ -569,12 +655,12 @@ x86FeatureLoad(xmlXPathContextPtr ctxt, { xmlNodePtr *nodes = NULL; xmlNodePtr ctxt_node = ctxt->node; - struct x86_feature *feature = NULL; + struct x86_feature *feature; int ret = 0; int i; int n; - if (VIR_ALLOC(feature) < 0) + if (!(feature = x86FeatureNew())) goto no_memory; feature->name = virXPathString("string(@name)", ctxt); @@ -594,14 +680,8 @@ x86FeatureLoad(xmlXPathContextPtr ctxt, if (n < 0) goto ignore; - if (n > 0) { - if (VIR_ALLOC_N(feature->cpuid, n) < 0) - goto no_memory; - feature->ncpuid = n; - } - for (i = 0; i < n; i++) { - struct cpuX86cpuid *cpuid = feature->cpuid + i; + struct cpuX86cpuid cpuid; unsigned long fun, eax, ebx, ecx, edx; int ret_fun, ret_eax, ret_ebx, ret_ecx, ret_edx; @@ -620,11 +700,14 @@ x86FeatureLoad(xmlXPathContextPtr ctxt, goto ignore; } - cpuid->function = fun; - cpuid->eax = eax; - cpuid->ebx = ebx; - cpuid->ecx = ecx; - cpuid->edx = edx; + cpuid.function = fun; + cpuid.eax = eax; + cpuid.ebx = ebx; + cpuid.ecx = ecx; + cpuid.edx = edx; + + if (x86DataAddCpuid(feature->data, &cpuid)) + goto no_memory; } if (map->features == NULL) @@ -650,6 +733,23 @@ ignore: } +static struct x86_model * +x86ModelNew(void) +{ + struct x86_model *model; + + if (VIR_ALLOC(model) < 0) + return NULL; + + if (VIR_ALLOC(model->data) < 0) { + VIR_FREE(model); + return NULL; + } + + return model; +} + + static void x86ModelFree(struct x86_model *model) { @@ -657,7 +757,7 @@ x86ModelFree(struct x86_model *model) return; VIR_FREE(model->name); - VIR_FREE(model->cpuid); + x86DataFree(model->data); VIR_FREE(model); } @@ -666,99 +766,20 @@ static struct x86_model * x86ModelCopy(const struct x86_model *model) { struct x86_model *copy; - int i; if (VIR_ALLOC(copy) < 0 - || (copy->name = strdup(model->name)) == NULL - || VIR_ALLOC_N(copy->cpuid, model->ncpuid) < 0) { + || !(copy->name = strdup(model->name)) + || !(copy->data = x86DataCopy(model->data))) { x86ModelFree(copy); return NULL; } copy->vendor = model->vendor; - copy->ncpuid = model->ncpuid; - for (i = 0; i < model->ncpuid; i++) - copy->cpuid[i] = model->cpuid[i]; return copy; } -static int -x86ModelAddCpuid(struct x86_model *model, - const struct cpuX86cpuid *cpuid) -{ - struct cpuX86cpuid *model_cpuid; - - model_cpuid = x86cpuidFind(model->cpuid, model->ncpuid, cpuid->function); - - if (model_cpuid != NULL) - x86cpuidSetBits(model_cpuid, cpuid); - else { - if (VIR_REALLOC_N(model->cpuid, model->ncpuid + 1) < 0) - return -1; - - model->cpuid[model->ncpuid] = *cpuid; - model->ncpuid++; - } - - return 0; -} - - -static void -x86ModelSubtract(struct x86_model *model1, - const struct x86_model *model2) -{ - int i; - struct cpuX86cpuid *cpuid; - - for (i = 0; i < model2->ncpuid; i++) { - cpuid = x86cpuidFind(model1->cpuid, - model1->ncpuid, - model2->cpuid[i].function); - if (cpuid != NULL) - x86cpuidClearBits(cpuid, model2->cpuid + i); - } -} - - -static void -x86ModelIntersect(struct x86_model *model1, - const struct x86_model *model2) -{ - int i; - struct cpuX86cpuid *cpuid; - - for (i = 0; i < model1->ncpuid; i++) { - struct cpuX86cpuid *intersection = model1->cpuid + i; - - cpuid = x86cpuidFind(model2->cpuid, - model2->ncpuid, - intersection->function); - if (cpuid != NULL) - x86cpuidAndBits(intersection, cpuid); - else - x86cpuidClearBits(intersection, intersection); - } -} - - -static int -x86ModelAdd(struct x86_model *model1, - const struct x86_model *model2) -{ - int i; - - for (i = 0; i < model2->ncpuid; i++) { - if (x86ModelAddCpuid(model1, model2->cpuid + i)) - return -1; - } - - return 0; -} - - static struct x86_model * x86ModelFind(const struct x86_map *map, const char *name) @@ -777,47 +798,6 @@ x86ModelFind(const struct x86_map *map, } -static int -x86ModelMergeFeature(struct x86_model *model, - const struct x86_feature *feature) -{ - int i; - - if (feature == NULL) - return 0; - - for (i = 0; i < feature->ncpuid; i++) { - if (x86ModelAddCpuid(model, feature->cpuid + i)) - return -1; - } - - return 0; -} - - -static bool -x86ModelHasFeature(struct x86_model *model, - const struct x86_feature *feature) -{ - unsigned int i; - struct cpuX86cpuid *cpuid; - struct cpuX86cpuid *model_cpuid; - - if (feature == NULL) - return false; - - for (i = 0; i < feature->ncpuid; i++) { - cpuid = feature->cpuid + i; - model_cpuid = x86cpuidFind(model->cpuid, model->ncpuid, - cpuid->function); - if (!model_cpuid || !x86cpuidMatchMasked(model_cpuid, cpuid)) - return false; - } - - return true; -} - - static struct x86_model * x86ModelFromCPU(const virCPUDefPtr cpu, const struct x86_map *map, @@ -835,11 +815,11 @@ x86ModelFromCPU(const virCPUDefPtr cpu, if ((model = x86ModelCopy(model)) == NULL) goto no_memory; - } - else if (VIR_ALLOC(model) < 0) + } else if (!(model = x86ModelNew())) { goto no_memory; - else if (cpu->type == VIR_CPU_TYPE_HOST) + } else if (cpu->type == VIR_CPU_TYPE_HOST) { return model; + } for (i = 0; i < cpu->nfeatures; i++) { const struct x86_feature *feature; @@ -854,7 +834,7 @@ x86ModelFromCPU(const virCPUDefPtr cpu, goto error; } - if (x86ModelMergeFeature(model, feature)) + if (x86DataAdd(model->data, feature->data)) goto no_memory; } @@ -884,11 +864,10 @@ x86ModelSubtractCPU(struct x86_model *model, return -1; } - x86ModelSubtract(model, cpu_model); + x86DataSubtract(model->data, cpu_model->data); for (i = 0; i < cpu->nfeatures; i++) { const struct x86_feature *feature; - unsigned int j; if (!(feature = x86FeatureFind(map, cpu->features[i].name))) { virCPUReportError(VIR_ERR_INTERNAL_ERROR, @@ -897,13 +876,7 @@ x86ModelSubtractCPU(struct x86_model *model, return -1; } - for (j = 0; j < feature->ncpuid; j++) { - struct cpuX86cpuid *cpuid; - cpuid = x86cpuidFind(model->cpuid, model->ncpuid, - feature->cpuid[j].function); - if (cpuid) - x86cpuidClearBits(cpuid, feature->cpuid + j); - } + x86DataSubtract(model->data, feature->data); } return 0; @@ -915,18 +888,15 @@ x86ModelCompare(const struct x86_model *model1, const struct x86_model *model2) { enum compare_result result = EQUAL; + struct data_iterator iter1 = DATA_ITERATOR_INIT(model1->data); + struct data_iterator iter2 = DATA_ITERATOR_INIT(model2->data); struct cpuX86cpuid *cpuid1; struct cpuX86cpuid *cpuid2; - int i; - for (i = 0; i < model1->ncpuid; i++) { + while ((cpuid1 = x86DataCpuidNext(&iter1))) { enum compare_result match = SUPERSET; - cpuid1 = model1->cpuid + i; - cpuid2 = x86cpuidFind(model2->cpuid, - model2->ncpuid, - cpuid1->function); - if (cpuid2 != NULL) { + if ((cpuid2 = x86DataCpuid(model2->data, cpuid1->function))) { if (x86cpuidMatch(cpuid1, cpuid2)) continue; else if (!x86cpuidMatchMasked(cpuid1, cpuid2)) @@ -939,14 +909,10 @@ x86ModelCompare(const struct x86_model *model1, return UNRELATED; } - for (i = 0; i < model2->ncpuid; i++) { + while ((cpuid2 = x86DataCpuidNext(&iter2))) { enum compare_result match = SUBSET; - cpuid2 = model2->cpuid + i; - cpuid1 = x86cpuidFind(model1->cpuid, - model1->ncpuid, - cpuid2->function); - if (cpuid1 != NULL) { + if ((cpuid1 = x86DataCpuid(model1->data, cpuid2->function))) { if (x86cpuidMatch(cpuid2, cpuid1)) continue; else if (!x86cpuidMatchMasked(cpuid2, cpuid1)) @@ -968,13 +934,13 @@ x86ModelLoad(xmlXPathContextPtr ctxt, struct x86_map *map) { xmlNodePtr *nodes = NULL; - struct x86_model *model = NULL; + struct x86_model *model; char *vendor = NULL; int ret = 0; int i; int n; - if (VIR_ALLOC(model) < 0) + if (!(model = x86ModelNew())) goto no_memory; model->name = virXPathString("string(@name)", ctxt); @@ -1006,13 +972,9 @@ x86ModelLoad(xmlXPathContextPtr ctxt, VIR_FREE(name); - if (VIR_ALLOC_N(model->cpuid, ancestor->ncpuid) < 0) - goto no_memory; - model->vendor = ancestor->vendor; - model->ncpuid = ancestor->ncpuid; - memcpy(model->cpuid, ancestor->cpuid, - sizeof(*model->cpuid) * model->ncpuid); + if (!(model->data = x86DataCopy(ancestor->data))) + goto no_memory; } if (virXPathBoolean("boolean(./vendor)", ctxt)) { @@ -1055,7 +1017,7 @@ x86ModelLoad(xmlXPathContextPtr ctxt, } VIR_FREE(name); - if (x86ModelMergeFeature(model, feature)) + if (x86DataAdd(model->data, feature->data)) goto no_memory; } @@ -1157,7 +1119,6 @@ x86Compute(virCPUDefPtr host, virCPUDefPtr cpu, union cpuData **guest) { - struct cpuX86cpuid cpuid_zero = { 0, 0, 0, 0, 0 }; struct x86_map *map = NULL; struct x86_model *host_model = NULL; struct x86_model *cpu_force = NULL; @@ -1167,6 +1128,8 @@ x86Compute(virCPUDefPtr host, struct x86_model *cpu_forbid = NULL; struct x86_model *diff = NULL; struct x86_model *guest_model = NULL; + struct data_iterator iter; + const struct cpuX86cpuid *cpuid; virCPUCompareResult ret; enum compare_result result; unsigned int i; @@ -1203,24 +1166,20 @@ x86Compute(virCPUDefPtr host, !(cpu_forbid = x86ModelFromCPU(cpu, map, VIR_CPU_FEATURE_FORBID))) goto error; - for (i = 0; i < cpu_forbid->ncpuid; i++) { - const struct cpuX86cpuid *cpuid1; + x86DataIteratorInit(&iter, cpu_forbid->data); + while ((cpuid = x86DataCpuidNext(&iter))) { const struct cpuX86cpuid *cpuid2; - cpuid1 = cpu_forbid->cpuid + i; - cpuid2 = x86cpuidFind(host_model->cpuid, - host_model->ncpuid, - cpuid1->function); - - if (cpuid2 != NULL && x86cpuidMatchAny(cpuid2, cpuid1)) { + cpuid2 = x86DataCpuid(host_model->data, cpuid->function); + if (cpuid2 != NULL && x86cpuidMatchAny(cpuid2, cpuid)) { VIR_DEBUG("Host CPU provides forbidden features in CPUID function 0x%x", - cpuid1->function); + cpuid->function); ret = VIR_CPU_COMPARE_INCOMPATIBLE; goto out; } } - x86ModelSubtract(cpu_require, cpu_disable); + x86DataSubtract(cpu_require->data, cpu_disable->data); result = x86ModelCompare(host_model, cpu_require); if (result == SUBSET || result == UNRELATED) { VIR_DEBUG0("Host CPU does not provide all required features"); @@ -1233,17 +1192,13 @@ x86Compute(virCPUDefPtr host, if ((diff = x86ModelCopy(host_model)) == NULL) goto no_memory; - x86ModelSubtract(diff, cpu_optional); - x86ModelSubtract(diff, cpu_require); - x86ModelSubtract(diff, cpu_disable); - x86ModelSubtract(diff, cpu_force); + x86DataSubtract(diff->data, cpu_optional->data); + x86DataSubtract(diff->data, cpu_require->data); + x86DataSubtract(diff->data, cpu_disable->data); + x86DataSubtract(diff->data, cpu_force->data); - for (i = 0; i < diff->ncpuid; i++) { - if (!x86cpuidMatch(diff->cpuid + i, &cpuid_zero)) { - ret = VIR_CPU_COMPARE_SUPERSET; - break; - } - } + if (!x86DataIsEmpty(diff->data)) + ret = VIR_CPU_COMPARE_SUPERSET; if (ret == VIR_CPU_COMPARE_SUPERSET && cpu->type == VIR_CPU_TYPE_GUEST @@ -1259,14 +1214,14 @@ x86Compute(virCPUDefPtr host, if (cpu->type == VIR_CPU_TYPE_GUEST && cpu->match == VIR_CPU_MATCH_EXACT) - x86ModelSubtract(guest_model, diff); + x86DataSubtract(guest_model->data, diff->data); - if (x86ModelAdd(guest_model, cpu_force)) + if (x86DataAdd(guest_model->data, cpu_force->data)) goto no_memory; - x86ModelSubtract(guest_model, cpu_disable); + x86DataSubtract(guest_model->data, cpu_disable->data); - if ((*guest = x86DataFromModel(guest_model)) == NULL) + if ((*guest = x86DataCopy(guest_model->data)) == NULL) goto no_memory; } @@ -1418,9 +1373,8 @@ x86EncodePolicy(const virCPUDefPtr cpu, if (!(model = x86ModelFromCPU(cpu, map, policy))) return NULL; - if (!(data = x86DataFromModel(model))) - virReportOOMError(); - + data = model->data; + model->data = NULL; x86ModelFree(model); return data; @@ -1584,19 +1538,20 @@ static union cpuData * x86NodeData(void) { union cpuData *data; + int ret; if (VIR_ALLOC(data) < 0) { virReportOOMError(); return NULL; } - data->x86.basic_len = cpuidSet(CPUX86_BASIC, &data->x86.basic); - if (data->x86.basic_len < 0) + if ((ret = cpuidSet(CPUX86_BASIC, &data->x86.basic)) < 0) goto error; + data->x86.basic_len = ret; - data->x86.extended_len = cpuidSet(CPUX86_EXTENDED, &data->x86.extended); - if (data->x86.extended_len < 0) + if ((ret = cpuidSet(CPUX86_EXTENDED, &data->x86.extended)) < 0) goto error; + data->x86.extended_len = ret; return data; @@ -1616,7 +1571,6 @@ x86Baseline(virCPUDefPtr *cpus, { struct x86_map *map = NULL; struct x86_model *base_model = NULL; - union cpuData *data = NULL; virCPUDefPtr cpu = NULL; unsigned int i; const struct x86_vendor *vendor = NULL; @@ -1679,24 +1633,21 @@ x86Baseline(virCPUDefPtr *cpus, } } - x86ModelIntersect(base_model, model); + x86DataIntersect(base_model->data, model->data); x86ModelFree(model); model = NULL; } - if (!(data = x86DataFromModel(base_model))) - goto no_memory; - - if (x86DataIsEmpty(data)) { + if (x86DataIsEmpty(base_model->data)) { virCPUReportError(VIR_ERR_OPERATION_FAILED, "%s", _("CPUs are incompatible")); goto error; } - if (vendor && x86DataAddCpuid(data, &vendor->cpuid) < 0) + if (vendor && x86DataAddCpuid(base_model->data, &vendor->cpuid) < 0) goto no_memory; - if (x86Decode(cpu, data, models, nmodels, NULL) < 0) + if (x86Decode(cpu, base_model->data, models, nmodels, NULL) < 0) goto error; if (!outputVendor) @@ -1705,7 +1656,6 @@ x86Baseline(virCPUDefPtr *cpus, VIR_FREE(cpu->arch); cleanup: - x86DataFree(data); x86ModelFree(base_model); x86MapFree(map); @@ -1729,7 +1679,6 @@ x86Update(virCPUDefPtr guest, unsigned int i; struct x86_map *map; struct x86_model *host_model = NULL; - union cpuData *data = NULL; if (!(map = x86LoadMap()) || !(host_model = x86ModelFromCPU(host, map, VIR_CPU_FEATURE_REQUIRE))) @@ -1745,7 +1694,7 @@ x86Update(virCPUDefPtr guest, goto cleanup; } - if (x86ModelHasFeature(host_model, feature)) + if (x86DataIsSubset(host_model->data, feature->data)) guest->features[i].policy = VIR_CPU_FEATURE_REQUIRE; else guest->features[i].policy = VIR_CPU_FEATURE_DISABLE; @@ -1755,8 +1704,8 @@ x86Update(virCPUDefPtr guest, if (guest->match == VIR_CPU_MATCH_MINIMUM) { guest->match = VIR_CPU_MATCH_EXACT; if (x86ModelSubtractCPU(host_model, guest, map) - || !(data = x86DataFromModel(host_model)) - || x86DataToCPUFeatures(guest, VIR_CPU_FEATURE_REQUIRE, data, map)) + || x86DataToCPUFeatures(guest, VIR_CPU_FEATURE_REQUIRE, + host_model->data, map)) goto cleanup; } @@ -1765,7 +1714,6 @@ x86Update(virCPUDefPtr guest, cleanup: x86MapFree(map); x86ModelFree(host_model); - x86DataFree(data); return ret; } @@ -1775,7 +1723,6 @@ static int x86HasFeature(const union cpuData *data, struct x86_map *map; struct x86_feature *feature; int ret = -1; - int i; if (!(map = x86LoadMap())) return -1; @@ -1783,16 +1730,7 @@ static int x86HasFeature(const union cpuData *data, if (!(feature = x86FeatureFind(map, name))) goto cleanup; - for (i = 0 ; i < feature->ncpuid ; i++) { - struct cpuX86cpuid *cpuid; - - cpuid = x86DataCpuid(data, feature->cpuid[i].function); - if (cpuid && x86cpuidMatchMasked(cpuid, feature->cpuid + i)) { - ret = 1; - goto cleanup; - } - } - ret = 0; + ret = x86DataIsSubset(data, feature->data) ? 1 : 0; cleanup: x86MapFree(map); diff --git a/src/cpu/cpu_x86_data.h b/src/cpu/cpu_x86_data.h index 46d2517..34a5b06 100644 --- a/src/cpu/cpu_x86_data.h +++ b/src/cpu/cpu_x86_data.h @@ -38,9 +38,9 @@ struct cpuX86cpuid { # define CPUX86_EXTENDED 0x80000000 struct cpuX86Data { - int basic_len; + size_t basic_len; struct cpuX86cpuid *basic; - int extended_len; + size_t extended_len; struct cpuX86cpuid *extended; }; -- 1.7.3.3

2 2

[libvirt] [PATCH] esx: Add support for storage volume cloning
by Matthias Bolte 14 Dec '10

14 Dec '10

--- src/esx/esx_storage_driver.c | 198 +++++++++++++++++++++++++++++++++++++++- src/esx/esx_vi_generator.input | 11 ++ 2 files changed, 208 insertions(+), 1 deletions(-) diff --git a/src/esx/esx_storage_driver.c b/src/esx/esx_storage_driver.c index 9165f7a..e6803c2 100644 --- a/src/esx/esx_storage_driver.c +++ b/src/esx/esx_storage_driver.c @@ -1180,6 +1180,202 @@ esxStorageVolumeCreateXML(virStoragePoolPtr pool, const char *xmldesc, +static virStorageVolPtr +esxStorageVolumeCreateXMLFrom(virStoragePoolPtr pool, const char *xmldesc, + virStorageVolPtr sourceVolume, unsigned int flags) +{ + virStorageVolPtr volume = NULL; + esxPrivate *priv = pool->conn->storagePrivateData; + virStoragePoolDef poolDef; + char *sourceDatastorePath = NULL; + virStorageVolDefPtr def = NULL; + char *tmp; + char *unescapedDatastorePath = NULL; + char *unescapedDirectoryName = NULL; + char *unescapedDirectoryAndFileName = NULL; + char *directoryName = NULL; + char *fileName = NULL; + char *datastorePathWithoutFileName = NULL; + char *datastorePath = NULL; + esxVI_FileInfo *fileInfo = NULL; + esxVI_ManagedObjectReference *task = NULL; + esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; + char *uuid_string = NULL; + char *key = NULL; + + virCheckFlags(0, NULL); + + memset(&poolDef, 0, sizeof (poolDef)); + + if (esxVI_EnsureSession(priv->primary) < 0) { + return NULL; + } + + if (esxStoragePoolLookupType(priv->primary, pool->name, &poolDef.type) < 0) { + return NULL; + } + + if (virAsprintf(&sourceDatastorePath, "[%s] %s", sourceVolume->pool, + sourceVolume->name) < 0) { + virReportOOMError(); + goto cleanup; + } + + /* Parse config */ + def = virStorageVolDefParseString(&poolDef, xmldesc); + + if (def == NULL) { + goto cleanup; + } + + if (def->type != VIR_STORAGE_VOL_FILE) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", + _("Creating non-file volumes is not supported")); + goto cleanup; + } + + /* Validate config */ + tmp = strrchr(def->name, '/'); + + if (tmp == NULL || *def->name == '/' || tmp[1] == '\0') { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Volume name '%s' doesn't have expected format " + "'<directory>/<file>'"), def->name); + goto cleanup; + } + + if (! virFileHasSuffix(def->name, ".vmdk")) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Volume name '%s' has unsupported suffix, expecting '.vmdk'"), + def->name); + goto cleanup; + } + + if (virAsprintf(&unescapedDatastorePath, "[%s] %s", pool->name, + def->name) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (def->target.format == VIR_STORAGE_FILE_VMDK) { + /* Parse and escape datastore path */ + if (esxUtil_ParseDatastorePath(unescapedDatastorePath, NULL, + &unescapedDirectoryName, + &unescapedDirectoryAndFileName) < 0) { + goto cleanup; + } + + directoryName = esxUtil_EscapeDatastoreItem(unescapedDirectoryName); + + if (directoryName == NULL) { + goto cleanup; + } + + fileName = esxUtil_EscapeDatastoreItem(unescapedDirectoryAndFileName + + strlen(unescapedDirectoryName) + 1); + + if (fileName == NULL) { + goto cleanup; + } + + if (virAsprintf(&datastorePathWithoutFileName, "[%s] %s", pool->name, + directoryName) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (virAsprintf(&datastorePath, "[%s] %s/%s", pool->name, directoryName, + fileName) < 0) { + virReportOOMError(); + goto cleanup; + } + + /* Create directory, if it doesn't exist yet */ + if (esxVI_LookupFileInfoByDatastorePath + (priv->primary, datastorePathWithoutFileName, true, &fileInfo, + esxVI_Occurrence_OptionalItem) < 0) { + goto cleanup; + } + + if (fileInfo == NULL) { + if (esxVI_MakeDirectory(priv->primary, datastorePathWithoutFileName, + priv->primary->datacenter->_reference, + esxVI_Boolean_True) < 0) { + goto cleanup; + } + } + + /* Copy VirtualDisk */ + if (esxVI_CopyVirtualDisk_Task(priv->primary, sourceDatastorePath, + priv->primary->datacenter->_reference, + datastorePath, + priv->primary->datacenter->_reference, + NULL, esxVI_Boolean_False, &task) < 0 || + esxVI_WaitForTaskCompletion(priv->primary, task, NULL, + esxVI_Occurrence_None, + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { + goto cleanup; + } + + if (taskInfoState != esxVI_TaskInfoState_Success) { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("Could not copy volume: %s"), + taskInfoErrorMessage); + goto cleanup; + } + + if (priv->primary->hasQueryVirtualDiskUuid) { + if (VIR_ALLOC_N(key, VIR_UUID_STRING_BUFLEN) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (esxVI_QueryVirtualDiskUuid(priv->primary, datastorePath, + priv->primary->datacenter->_reference, + &uuid_string) < 0) { + goto cleanup; + } + + if (esxUtil_ReformatUuid(uuid_string, key) < 0) { + goto cleanup; + } + } else { + /* Fall back to the path as key */ + if (esxVI_String_DeepCopyValue(&key, datastorePath) < 0) { + goto cleanup; + } + } + } else { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Creation of %s volumes is not supported"), + virStorageFileFormatTypeToString(def->target.format)); + goto cleanup; + } + + volume = virGetStorageVol(pool->conn, pool->name, def->name, key); + + cleanup: + VIR_FREE(sourceDatastorePath); + virStorageVolDefFree(def); + VIR_FREE(unescapedDatastorePath); + VIR_FREE(unescapedDirectoryName); + VIR_FREE(unescapedDirectoryAndFileName); + VIR_FREE(directoryName); + VIR_FREE(fileName); + VIR_FREE(datastorePathWithoutFileName); + VIR_FREE(datastorePath); + esxVI_FileInfo_Free(&fileInfo); + esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); + VIR_FREE(uuid_string); + VIR_FREE(key); + + return volume; +} + + + static int esxStorageVolumeGetInfo(virStorageVolPtr volume, virStorageVolInfoPtr info) { @@ -1377,7 +1573,7 @@ static virStorageDriver esxStorageDriver = { esxStorageVolumeLookupByKey, /* volLookupByKey */ esxStorageVolumeLookupByPath, /* volLookupByPath */ esxStorageVolumeCreateXML, /* volCreateXML */ - NULL, /* volCreateXMLFrom */ + esxStorageVolumeCreateXMLFrom, /* volCreateXMLFrom */ NULL, /* volDelete */ NULL, /* volWipe */ esxStorageVolumeGetInfo, /* volGetInfo */ diff --git a/src/esx/esx_vi_generator.input b/src/esx/esx_vi_generator.input index 991ce8a..4018c6e 100644 --- a/src/esx/esx_vi_generator.input +++ b/src/esx/esx_vi_generator.input @@ -700,6 +700,17 @@ method CancelTask end +method CopyVirtualDisk_Task returns ManagedObjectReference r + ManagedObjectReference _this:VirtualDiskManager r + String sourceName r + ManagedObjectReference sourceDatacenter o + String destName r + ManagedObjectReference destDatacenter o + VirtualDiskSpec destSpec o + Boolean force o +end + + method CreateFilter returns ManagedObjectReference r ManagedObjectReference _this:PropertyCollector r PropertyFilterSpec spec r -- 1.7.0.4

2 2

[libvirt] [PATCH] qemu: Use -vga none only if it is supported
by Jiri Denemark 14 Dec '10

14 Dec '10

Commit febc591683cf51e4551f8bcf3ce279a776056e1c introduced -vga none in case no video card is included in domain XML. However, old qemu versions do not support this and such domain cannot be successfully started. --- src/qemu/qemu_conf.c | 10 ++++++- src/qemu/qemu_conf.h | 1 + tests/qemuhelptest.c | 18 +++++++++---- .../qemuxml2argvdata/qemuxml2argv-nographics.args | 1 + tests/qemuxml2argvdata/qemuxml2argv-nographics.xml | 25 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 4 ++- 6 files changed, 50 insertions(+), 9 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-nographics.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-nographics.xml diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index e5d0206..d5e72cd 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1220,6 +1220,7 @@ static unsigned long long qemudComputeCmdFlags(const char *help, unsigned int kvm_version) { unsigned long long flags = 0; + const char *p; if (strstr(help, "-no-kqemu")) flags |= QEMUD_CMD_FLAG_KQEMU; @@ -1252,11 +1253,15 @@ static unsigned long long qemudComputeCmdFlags(const char *help, if (strstr(help, "readonly=")) flags |= QEMUD_CMD_FLAG_DRIVE_READONLY; } - if (strstr(help, "-vga") && !strstr(help, "-std-vga")) { + if ((p = strstr(help, "-vga")) && !strstr(help, "-std-vga")) { + const char *nl = strstr(p, "\n"); + flags |= QEMUD_CMD_FLAG_VGA; if (strstr(help, "|qxl")) flags |= QEMUD_CMD_FLAG_VGA_QXL; + if ((p = strstr(p, "|none")) && p < nl) + flags |= QEMUD_CMD_FLAG_VGA_NONE; } if (strstr(help, "-spice")) flags |= QEMUD_CMD_FLAG_SPICE; @@ -5192,7 +5197,8 @@ qemudBuildCommandLine(virConnectPtr conn, } else { /* If we have -device, then we set -nodefault already */ if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) && - (qemuCmdFlags & QEMUD_CMD_FLAG_VGA)) + (qemuCmdFlags & QEMUD_CMD_FLAG_VGA) && + (qemuCmdFlags & QEMUD_CMD_FLAG_VGA_NONE)) virCommandAddArgList(cmd, "-vga", "none", NULL); } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 71318bf..2d0314c 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -101,6 +101,7 @@ enum qemud_cmd_flags { QEMUD_CMD_FLAG_SMBIOS_TYPE = (1LL << 44), /* Is -smbios type= available */ QEMUD_CMD_FLAG_VGA_QXL = (1LL << 45), /* The 'qxl' arg for '-vga' */ QEMUD_CMD_FLAG_SPICE = (1LL << 46), /* Is -spice avail */ + QEMUD_CMD_FLAG_VGA_NONE = (1LL << 47), /* The 'none' arg for '-vga' */ }; /* Main driver state */ diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index 5339ec8..f3b6537 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -173,7 +173,8 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_ENABLE_KVM | QEMUD_CMD_FLAG_SDL | QEMUD_CMD_FLAG_RTC_TD_HACK | - QEMUD_CMD_FLAG_NO_HPET, + QEMUD_CMD_FLAG_NO_HPET | + QEMUD_CMD_FLAG_VGA_NONE, 10005, 0, 0); DO_TEST("qemu-kvm-0.10.5", QEMUD_CMD_FLAG_VNC_COLON | @@ -198,7 +199,8 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_NO_HPET | QEMUD_CMD_FLAG_NO_KVM_PIT | QEMUD_CMD_FLAG_TDF | - QEMUD_CMD_FLAG_NESTING, + QEMUD_CMD_FLAG_NESTING | + QEMUD_CMD_FLAG_VGA_NONE, 10005, 1, 0); DO_TEST("kvm-86", QEMUD_CMD_FLAG_VNC_COLON | @@ -223,7 +225,8 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_NO_KVM_PIT | QEMUD_CMD_FLAG_TDF | QEMUD_CMD_FLAG_NESTING | - QEMUD_CMD_FLAG_SMBIOS_TYPE, + QEMUD_CMD_FLAG_SMBIOS_TYPE | + QEMUD_CMD_FLAG_VGA_NONE, 10050, 1, 0); DO_TEST("qemu-kvm-0.11.0-rc2", QEMUD_CMD_FLAG_VNC_COLON | @@ -253,7 +256,8 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_BOOT_MENU | QEMUD_CMD_FLAG_NESTING | QEMUD_CMD_FLAG_NAME_PROCESS | - QEMUD_CMD_FLAG_SMBIOS_TYPE, + QEMUD_CMD_FLAG_SMBIOS_TYPE | + QEMUD_CMD_FLAG_VGA_NONE, 10092, 1, 0); DO_TEST("qemu-0.12.1", QEMUD_CMD_FLAG_VNC_COLON | @@ -281,7 +285,8 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_NO_HPET | QEMUD_CMD_FLAG_BOOT_MENU | QEMUD_CMD_FLAG_NAME_PROCESS | - QEMUD_CMD_FLAG_SMBIOS_TYPE, + QEMUD_CMD_FLAG_SMBIOS_TYPE | + QEMUD_CMD_FLAG_VGA_NONE, 12001, 0, 0); DO_TEST("qemu-kvm-0.12.3", QEMUD_CMD_FLAG_VNC_COLON | @@ -316,7 +321,8 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_BOOT_MENU | QEMUD_CMD_FLAG_NESTING | QEMUD_CMD_FLAG_NAME_PROCESS | - QEMUD_CMD_FLAG_SMBIOS_TYPE, + QEMUD_CMD_FLAG_SMBIOS_TYPE | + QEMUD_CMD_FLAG_VGA_NONE, 12003, 1, 0); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-nographics.args b/tests/qemuxml2argvdata/qemuxml2argv-nographics.args new file mode 100644 index 0000000..0d3ef6f --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-nographics.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-nographics.xml b/tests/qemuxml2argvdata/qemuxml2argv-nographics.xml new file mode 100644 index 0000000..ed91e37 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-nographics.xml @@ -0,0 +1,25 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 5fe91f1..662f7bb 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -325,7 +325,9 @@ mymain(int argc, char **argv) DO_TEST("graphics-sdl", 0, false); DO_TEST("graphics-sdl-fullscreen", 0, false); - DO_TEST("nographics-vga", QEMUD_CMD_FLAG_VGA, false); + DO_TEST("nographics", QEMUD_CMD_FLAG_VGA, false); + DO_TEST("nographics-vga", QEMUD_CMD_FLAG_VGA | + QEMUD_CMD_FLAG_VGA_NONE, false); DO_TEST("graphics-spice", QEMUD_CMD_FLAG_VGA | QEMUD_CMD_FLAG_VGA_QXL | QEMUD_CMD_FLAG_DEVICE | QEMUD_CMD_FLAG_SPICE, false); -- 1.7.3.3

4 8

[libvirt] [PATCHv2 0/5] smbios cleanups
by Eric Blake 14 Dec '10

14 Dec '10

No change to the first three patches (but reposting so they'll all be in the same thread). The last two patches are new. Eric Blake (5): qemu: avoid adding "" in smbios arguments smbios: support system family smbios: allow () in smbios strings uuid: require smbios uuid and domain uuid to match sysinfo: convert to virCommand docs/schemas/domain.rng | 3 +- src/conf/domain_conf.c | 31 ++++++++++++- src/qemu/qemu_conf.c | 40 ++++++++++------ src/util/sysinfo.c | 57 +++++++---------------- src/util/sysinfo.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-smbios.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-smbios.xml | 20 ++++---- 7 files changed, 85 insertions(+), 69 deletions(-) -- 1.7.3.2

3 16

[libvirt] [PATCH] build: update gnulib for pipe on mingw
by Eric Blake 14 Dec '10

14 Dec '10

* .gnulib: Update to latest. * bootstrap.conf (gnulib_modules): Import pipe-posix for mingw. * src/remote/remote_driver.c (pipe): Drop dead macro. --- Matthias complianed on IRC that mingw builds are broken since they unconditionally build virCommand, which unconditionally uses pipe() and waitpid() even though those are missing on mingw. I've fixed pipe() in gnulib. waitpid() will need a bit more invasive fix; I'm thinking of creating: struct virPid { #ifdef WIN32 HANDLE pid; #else pid_t pid; #endif }; /* Wait for a child process to complete. */ int virPidWait(virPid pid, int *status) { #ifdef WIN32 whatever windows provides #else return waitpid(pid.pid, status, 0); #endif } or something like that. * .gnulib 6491120...e2f1471 (3): > pipe-posix: new module > * build-aux/gendocs.sh: restore x bit > autoupdate .gnulib | 2 +- bootstrap.conf | 1 + src/remote/remote_driver.c | 5 ----- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/.gnulib b/.gnulib index 6491120..e2f1471 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 64911207854610668b480939469282fdaeb96f74 +Subproject commit e2f1471b021a285916339a73bc12c6b44dbf9a76 diff --git a/bootstrap.conf b/bootstrap.conf index 2ad1957..dbed752 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -48,6 +48,7 @@ mktempd netdb perror physmem +pipe-posix poll posix-shell pthread diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index e6eb9b5..fae191c 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -86,11 +86,6 @@ #define VIR_FROM_THIS VIR_FROM_REMOTE -#ifdef WIN32 -# define pipe(fds) _pipe(fds,4096, _O_BINARY) -#endif - - static int inside_daemon = 0; struct remote_thread_call; -- 1.7.3.2

2 10

[libvirt] [PATCH] test: fix commantest under autobuild.sh
by Eric Blake 14 Dec '10

14 Dec '10

* tests/commandtest.c (mymain): Kill off any leaked-in fds. * autobuild.sh: Don't leak fds. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- Autobuild failed because it triggered a commandtest failure where commandtest was too sensitive to the environment. Fix the problem at both ends - autobuild shouldn't leak, and commandtest should work even when somebody else leaked. autobuild.sh | 2 +- tests/commandtest.c | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletions(-) diff --git a/autobuild.sh b/autobuild.sh index 8248a59..91e2ab2 100755 --- a/autobuild.sh +++ b/autobuild.sh @@ -38,7 +38,7 @@ make install exec 3>&1 st=$( exec 4>&1 >&3 - { make check syntax-check 2>&1; echo $? >&4; } | tee "$RESULTS" + { make check syntax-check 2>&1 3>&- 4>&-; echo $? >&4; } | tee "$RESULTS" ) exec 3>&- test "$st" = 0 diff --git a/tests/commandtest.c b/tests/commandtest.c index a1bcf68..333dd4d 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -688,6 +688,12 @@ mymain(int argc, char **argv) if (chdir("/tmp") < 0) return(EXIT_FAILURE); + /* Kill off any inherited fds that might interfere with our + * testing. */ + close(3); + close(4); + close(5); + virInitialize(); const char *const newenv[] = { -- 1.7.3.2

2 2

[libvirt] [v2] qemu: Introduce two new job types
by Osier Yang 13 Dec '10

13 Dec '10

Currently, all of domain "save/dump/managed save/migration" use the same function "qemudDomainWaitForMigrationComplete" to wait the job finished, but the error messages are all about "migration", e.g. when a domain saving job is canceled by user, "migration was cancled by client" will be throwed as an error message, which will be confused for user. As a solution, intoduce two new job types(QEMU_JOB_SAVE, QEMU_JOB_DUMP), and set "priv->jobActive" to "QEMU_JOB_SAVE" before saving, to "QEMU_JOB_DUMP" before dumping, so that we could get the real job type in "qemudDomainWaitForMigrationComplete", and give more clear message further. And as It's not important to figure out what's the exact job is in the DEBUG and WARN log, also we don't need translated string in logs, simply repace "migration" with "job" in some statements. * src/qemu/qemu_driver.c --- src/qemu/qemu_driver.c | 40 +++++++++++++++++++++++++++++++--------- 1 files changed, 31 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 19ce9a6..d5af0df 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -104,6 +104,8 @@ enum qemuDomainJob { QEMU_JOB_UNSPECIFIED, QEMU_JOB_MIGRATION_OUT, QEMU_JOB_MIGRATION_IN, + QEMU_JOB_SAVE, + QEMU_JOB_DUMP, }; enum qemuDomainJobSignals { @@ -5389,21 +5391,37 @@ qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, virDomainObjPtr struct timespec ts = { .tv_sec = 0, .tv_nsec = 50 * 1000 * 1000ull }; struct timeval now; int rc; + const char *job; + + switch (priv->jobActive) { + case QEMU_JOB_MIGRATION_OUT: + job = _("migration job"); + break; + case QEMU_JOB_SAVE: + job = _("domain save job"); + break; + case QEMU_JOB_DUMP: + job = _("domain core dump job"); + break; + default: + job = _("job"); + } + if (!virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("guest unexpectedly quit during migration")); + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s: %s", + job, _("guest unexpectedly quit")); goto cleanup; } if (priv->jobSignals & QEMU_JOB_SIGNAL_CANCEL) { priv->jobSignals ^= QEMU_JOB_SIGNAL_CANCEL; - VIR_DEBUG0("Cancelling migration at client request"); + VIR_DEBUG0("Cancelling job at client request"); qemuDomainObjEnterMonitorWithDriver(driver, vm); rc = qemuMonitorMigrateCancel(priv->mon); qemuDomainObjExitMonitorWithDriver(driver, vm); if (rc < 0) { - VIR_WARN0("Unable to cancel migration"); + VIR_WARN0("Unable to cancel job"); } } else if (priv->jobSignals & QEMU_JOB_SIGNAL_SUSPEND) { priv->jobSignals ^= QEMU_JOB_SIGNAL_SUSPEND; @@ -5427,8 +5445,8 @@ qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, virDomainObjPtr * guest to die */ if (!virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("guest unexpectedly quit during migration")); + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s: %s", + job, _("guest unexpectedly quit")); goto cleanup; } @@ -5459,7 +5477,7 @@ qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, virDomainObjPtr case QEMU_MONITOR_MIGRATION_STATUS_INACTIVE: priv->jobInfo.type = VIR_DOMAIN_JOB_NONE; qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("Migration is not active")); + "%s: %s", job, _("is not active")); break; case QEMU_MONITOR_MIGRATION_STATUS_ACTIVE: @@ -5480,13 +5498,13 @@ qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, virDomainObjPtr case QEMU_MONITOR_MIGRATION_STATUS_ERROR: priv->jobInfo.type = VIR_DOMAIN_JOB_FAILED; qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("Migration unexpectedly failed")); + "%s: %s", job, _("unexpectedly failed")); break; case QEMU_MONITOR_MIGRATION_STATUS_CANCELLED: priv->jobInfo.type = VIR_DOMAIN_JOB_CANCELLED; qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("Migration was cancelled by client")); + "%s: %s", job, _("canceled by client")); break; } @@ -5606,6 +5624,8 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, goto endjob; } + priv->jobActive = QEMU_JOB_SAVE; + memset(&priv->jobInfo, 0, sizeof(priv->jobInfo)); priv->jobInfo.type = VIR_DOMAIN_JOB_UNBOUNDED; @@ -6198,6 +6218,8 @@ static int qemudDomainCoreDump(virDomainPtr dom, goto endjob; } + priv->jobActive = QEMU_JOB_DUMP; + /* Migrate will always stop the VM, so the resume condition is independent of whether the stop command is issued. */ resume = (vm->state == VIR_DOMAIN_RUNNING); -- 1.7.3.2

2 1

[libvirt] Missing "Default-Stop" field in LSB comment in libvirt-guests
by Laurent Léonard 13 Dec '10

13 Dec '10

Hi, The "Default-Stop" field in LSB comment in libvirt-guests is missing and should be added. I also suggests to add runlevel 2 to the "Default-Start" field. The attached patch fixes that issue. -- Laurent Léonard

2 3

[libvirt] [PATCH] daemon: plug a memory leak
by Eric Blake 13 Dec '10

13 Dec '10

* daemon/libvirtd.c (qemudFreeClient): Avoid a leak. (qemudDispatchServer): Avoid null dereference. --- I keep finding more of these. daemon/libvirtd.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 791b3dc..2914f2f 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1410,7 +1410,7 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket /* Client is running as root, so disable auth */ if (uid == 0) { VIR_INFO(_("Turn off polkit auth for privileged client pid %d from %s"), - pid, addrstr); + pid, client->addrstr); client->auth = REMOTE_AUTH_NONE; } } @@ -1451,7 +1451,7 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket } else { PROBE(CLIENT_TLS_FAIL, "fd=%d", client->fd); VIR_ERROR(_("TLS handshake failed for client %s: %s"), - addrstr, gnutls_strerror (ret)); + client->addrstr, gnutls_strerror (ret)); goto error; } } @@ -2283,6 +2283,7 @@ static void qemudFreeClient(struct qemud_client *client) { if (client->conn) virConnectClose(client->conn); virMutexDestroy(&client->lock); + VIR_FREE(client->addrstr); VIR_FREE(client); } -- 1.7.3.2

3 3

[libvirt] [PATCH] qemu: Introduce two new job types
by Osier Yang 13 Dec '10

13 Dec '10

Currently, all of domain "save/dump/managed save/migration" use the same function "qemudDomainWaitForMigrationComplete" to wait the job finished, but the error messages are all about "migration", e.g. when a domain saving job is canceled by user, "migration was cancled by client" will be throwed as an error message, which will be confused for user. As a solution, intoduce two new job types(QEMU_JOB_SAVE, QEMU_JOB_DUMP), and set "priv->jobActive" to "QEMU_JOB_SAVE" before saving, to "QEMU_JOB_DUMP" before dumping, so that we could get the real job type in "qemudDomainWaitForMigrationComplete", and give more clear message further. * src/qemu/qemu_driver.c --- src/qemu/qemu_driver.c | 39 ++++++++++++++++++++++++++++++--------- 1 files changed, 30 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 19ce9a6..aed48f4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -104,6 +104,8 @@ enum qemuDomainJob { QEMU_JOB_UNSPECIFIED, QEMU_JOB_MIGRATION_OUT, QEMU_JOB_MIGRATION_IN, + QEMU_JOB_SAVE, + QEMU_JOB_DUMP, }; enum qemuDomainJobSignals { @@ -5389,21 +5391,36 @@ qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, virDomainObjPtr struct timespec ts = { .tv_sec = 0, .tv_nsec = 50 * 1000 * 1000ull }; struct timeval now; int rc; + const char *job; + + switch (priv->jobActive) { + case QEMU_JOB_MIGRATION_OUT: + job = "migration"; + break; + case QEMU_JOB_SAVE: + job = "domain saving"; + break; + case QEMU_JOB_DUMP: + job = "domain core dump"; + break; + default: + job = "job"; + } if (!virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("guest unexpectedly quit during migration")); + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("guest unexpectedly quit during %s"), job); goto cleanup; } if (priv->jobSignals & QEMU_JOB_SIGNAL_CANCEL) { priv->jobSignals ^= QEMU_JOB_SIGNAL_CANCEL; - VIR_DEBUG0("Cancelling migration at client request"); + VIR_DEBUG("Cancelling %s at client request", job); qemuDomainObjEnterMonitorWithDriver(driver, vm); rc = qemuMonitorMigrateCancel(priv->mon); qemuDomainObjExitMonitorWithDriver(driver, vm); if (rc < 0) { - VIR_WARN0("Unable to cancel migration"); + VIR_WARN("Unable to cancel %s", job); } } else if (priv->jobSignals & QEMU_JOB_SIGNAL_SUSPEND) { priv->jobSignals ^= QEMU_JOB_SIGNAL_SUSPEND; @@ -5427,8 +5444,8 @@ qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, virDomainObjPtr * guest to die */ if (!virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("guest unexpectedly quit during migration")); + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("guest unexpectedly quit during %s"), job); goto cleanup; } @@ -5459,7 +5476,7 @@ qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, virDomainObjPtr case QEMU_MONITOR_MIGRATION_STATUS_INACTIVE: priv->jobInfo.type = VIR_DOMAIN_JOB_NONE; qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("Migration is not active")); + _("%s is not active"), job); break; case QEMU_MONITOR_MIGRATION_STATUS_ACTIVE: @@ -5480,13 +5497,13 @@ qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, virDomainObjPtr case QEMU_MONITOR_MIGRATION_STATUS_ERROR: priv->jobInfo.type = VIR_DOMAIN_JOB_FAILED; qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("Migration unexpectedly failed")); + _("%s unexpectedly failed"), job); break; case QEMU_MONITOR_MIGRATION_STATUS_CANCELLED: priv->jobInfo.type = VIR_DOMAIN_JOB_CANCELLED; qemuReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("Migration was cancelled by client")); + _("%s was cancelled by client"), job); break; } @@ -5606,6 +5623,8 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, goto endjob; } + priv->jobActive = QEMU_JOB_SAVE; + memset(&priv->jobInfo, 0, sizeof(priv->jobInfo)); priv->jobInfo.type = VIR_DOMAIN_JOB_UNBOUNDED; @@ -6198,6 +6217,8 @@ static int qemudDomainCoreDump(virDomainPtr dom, goto endjob; } + priv->jobActive = QEMU_JOB_DUMP; + /* Migrate will always stop the VM, so the resume condition is independent of whether the stop command is issued. */ resume = (vm->state == VIR_DOMAIN_RUNNING); -- 1.7.3.2

2 4

[libvirt] ANNOUNCE: ruby-libvirt bindings 0.3.0
by Chris Lalancette 13 Dec '10

13 Dec '10

All, I'm pleased to announce the release of 0.3.0 of the ruby-libvirt bindings. This release has a number of updates: * Implementation of Libvirt::open_auth, Libvirt::event_register_impl * Updated Connect class, implementing conn.compare_cpu, conn.baseline_cpu, conn.domain_event_register_any, conn.domain_event_deregister_any, conn.domain_event_register, conn.domain_event_deregister, and conn.create_domain_xml. * Updated Domain class, implementing dom.get_vcpus, dom.update_device, dom.scheduler_type, dom.scheduler_parameters, dom.scheduler_parameters=, dom.num_vcpus, dom.vcpus_flags=, and dom.qemu_monitor_command. * Updated Interface class, implementing interface.free * Many potential memory leaks have been fixed. * Many bugfixes. * Documentation update of many methods, including all of the lookup methods that were missing before. With the above list, almost all of the APIs up to and including libvirt 0.8.6 are covered. There are a few that I have not yet had time to implement; you can look at the list in the README file in the source tree to see which ones are missing. The tarballs and source RPMs are here: http://libvirt.org/ruby/download/ The git repository is here: http://libvirt.org/git/?p=ruby-libvirt.git;a=summary The documentation for all of the APIs are here: http://libvirt.org/ruby/api/index.html I've uploaded some examples on how to use the bindings: http://libvirt.org/ruby/documentation.html I've also pushed a new gem to rubygems.org; you should be able to use 'gem install ruby-libvirt' (if you haven't installed them before) or 'gem update ruby-libvirt' (if you have them installed already). If you have time, and are interested in the bindings, please give these new ones a whirl and let me know if they work for you. Note that release 0.3.0 is intended to be fully backwards compatible with release 0.2.0 and 0.1.0. If you find that they are not, or find any other problems with the ruby bindings, please report it to the libvirt development mailing list (and CC me) so that I can take a look at them. Thanks, -- Chris Lalancette

1 0

[libvirt] [PATCH] Convert dhcpStartDhcpDaemon from virRun to virCommand
by Laine Stump 11 Dec '10

11 Dec '10

This is pretty straightforward - even though dnsmasq gets daemonized and uses a pid file, those things are both handled by the dnsmasq binary itself. And libvirt doesn't need any of the output of the dnsmasq command either, so we just setup the args and call virRun(). Mainly it was just a (mostly) mechanical job of replacing the APPEND_ARG() macro (and some other *printfs()) with virCommandAddArg*(). --- src/network/bridge_driver.c | 238 +++++++++++++++---------------------------- 1 files changed, 80 insertions(+), 158 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 62639e4..9802222 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -51,6 +51,7 @@ #include "event.h" #include "buf.h" #include "util.h" +#include "command.h" #include "memory.h" #include "uuid.h" #include "iptables.h" @@ -390,25 +391,15 @@ networkSaveDnsmasqHostsfile(virNetworkObjPtr network, static int networkBuildDnsmasqArgv(virNetworkObjPtr network, const char *pidfile, - const char ***argv) { - int i, len, r; + virCommandPtr cmd) { + int r, ret = -1; int nbleases = 0; - char *pidfileArg; - char buf[1024]; - unsigned int ranges; - char *ipAddr; - - /* - * For static-only DHCP, i.e. with no range but at least one host element, - * we have to add a special --dhcp-range option to enable the service in - * dnsmasq. - */ - ranges = network->def->nranges; - if (!ranges && network->def->nhosts) - ranges = 1; + char *bridgeaddr; + if (!(bridgeaddr = virSocketFormatAddr(&network->def->ipAddress))) + goto cleanup; /* - * NB, be careful about syntax for dnsmasq options in long format + * NB, be careful about syntax for dnsmasq options in long format. * * If the flag has a mandatory argument, it can be given using * either syntax: @@ -422,66 +413,27 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, * --foo=bar * * It is hard to determine whether a flag is optional or not, - * without reading the dnsmasq source :-( The manpages is not - * very explicit on this + * without reading the dnsmasq source :-( The manpage is not + * very explicit on this. */ - len = - 1 + /* dnsmasq */ - 1 + /* --strict-order */ - 1 + /* --bind-interfaces */ - (network->def->domain?2:0) + /* --domain name */ - 2 + /* --pid-file /var/run/libvirt/network/$NAME.pid */ - 2 + /* --conf-file "" */ - /*2 + *//* --interface virbr0 */ - 2 + /* --except-interface lo */ - 2 + /* --listen-address 10.0.0.1 */ - (2 * ranges) + /* --dhcp-range 10.0.0.2,10.0.0.254 */ - /* --dhcp-lease-max=xxx if needed */ - (network->def->nranges ? 1 : 0) + - /* --dhcp-no-override if needed */ - (ranges ? 1 : 0) + - /* --dhcp-hostsfile=/var/lib/dnsmasq/$NAME.hostsfile */ - (network->def->nhosts > 0 ? 1 : 0) + - /* --enable-tftp --tftp-root /srv/tftp */ - (network->def->tftproot ? 3 : 0) + - /* --dhcp-boot pxeboot.img[,,12.34.56.78] */ - (network->def->bootfile ? 2 : 0) + - 1; /* NULL */ - - if (VIR_ALLOC_N(*argv, len) < 0) - goto no_memory; - -#define APPEND_ARG(v, n, s) do { \ - if (!((v)[(n)] = strdup(s))) \ - goto no_memory; \ - } while (0) - -#define APPEND_ARG_LIT(v, n, s) \ - (v)[(n)] = s - - i = 0; - - APPEND_ARG(*argv, i++, DNSMASQ); - /* * Needed to ensure dnsmasq uses same algorithm for processing * multiple namedriver entries in /etc/resolv.conf as GLibC. */ - APPEND_ARG(*argv, i++, "--strict-order"); - APPEND_ARG(*argv, i++, "--bind-interfaces"); + virCommandAddArg(cmd, "--strict-order"); + virCommandAddArg(cmd, "--bind-interfaces"); if (network->def->domain) { - APPEND_ARG(*argv, i++, "--domain"); - APPEND_ARG(*argv, i++, network->def->domain); + virCommandAddArg(cmd, "--domain"); + virCommandAddArg(cmd, network->def->domain); } - if (virAsprintf(&pidfileArg, "--pid-file=%s", pidfile) < 0) - goto no_memory; - APPEND_ARG_LIT(*argv, i++, pidfileArg); + virCommandAddArgPair(cmd, "--pid-file", pidfile); - APPEND_ARG(*argv, i++, "--conf-file="); - APPEND_ARG(*argv, i++, ""); + /* *no* conf file */ + virCommandAddArg(cmd, "--conf-file="); + virCommandAddArg(cmd, ""); /* * XXX does not actually work, due to some kind of @@ -489,166 +441,140 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, * interface. A sleep(10) makes it work, but that's * clearly not practical * - * APPEND_ARG(*argv, i++, "--interface"); - * APPEND_ARG(*argv, i++, network->def->bridge); + * virCommandAddArg(cmd, "--interface"); + * virCommandAddArg(cmd, network->def->bridge); */ - APPEND_ARG(*argv, i++, "--listen-address"); - if (!(ipAddr = virSocketFormatAddr(&network->def->ipAddress))) - goto error; - APPEND_ARG_LIT(*argv, i++, ipAddr); + virCommandAddArg(cmd, "--listen-address"); + virCommandAddArg(cmd, bridgeaddr); - APPEND_ARG(*argv, i++, "--except-interface"); - APPEND_ARG(*argv, i++, "lo"); + virCommandAddArg(cmd, "--except-interface"); + virCommandAddArg(cmd, "lo"); for (r = 0 ; r < network->def->nranges ; r++) { char *saddr = virSocketFormatAddr(&network->def->ranges[r].start); if (!saddr) - goto error; + goto cleanup; char *eaddr = virSocketFormatAddr(&network->def->ranges[r].end); if (!eaddr) { VIR_FREE(saddr); - goto error; + goto cleanup; } - char *range; - int rc = virAsprintf(&range, "%s,%s", saddr, eaddr); + virCommandAddArg(cmd, "--dhcp-range"); + virCommandAddArgFormat(cmd, "%s,%s", saddr, eaddr); VIR_FREE(saddr); VIR_FREE(eaddr); - if (rc < 0) - goto no_memory; - APPEND_ARG(*argv, i++, "--dhcp-range"); - APPEND_ARG_LIT(*argv, i++, range); nbleases += virSocketGetRange(&network->def->ranges[r].start, &network->def->ranges[r].end); } + /* + * For static-only DHCP, i.e. with no range but at least one host element, + * we have to add a special --dhcp-range option to enable the service in + * dnsmasq. + */ if (!network->def->nranges && network->def->nhosts) { - char *ipaddr = virSocketFormatAddr(&network->def->ipAddress); - if (!ipaddr) - goto error; - char *range; - int rc = virAsprintf(&range, "%s,static", ipaddr); - VIR_FREE(ipaddr); - if (rc < 0) - goto no_memory; - - APPEND_ARG(*argv, i++, "--dhcp-range"); - APPEND_ARG_LIT(*argv, i++, range); + virCommandAddArg(cmd, "--dhcp-range"); + virCommandAddArgFormat(cmd, "%s,static", bridgeaddr); } if (network->def->nranges > 0) { - snprintf(buf, sizeof(buf), "--dhcp-lease-max=%d", nbleases); - APPEND_ARG(*argv, i++, buf); + virCommandAddArgFormat(cmd, "--dhcp-lease-max=%d", nbleases); } - if (ranges) - APPEND_ARG(*argv, i++, "--dhcp-no-override"); + if (network->def->nranges || network->def->nhosts) + virCommandAddArg(cmd, "--dhcp-no-override"); if (network->def->nhosts > 0) { - dnsmasqContext *dctx = dnsmasqContextNew(network->def->name, DNSMASQ_STATE_DIR); - char *hostsfileArg; - - if (dctx == NULL) - goto no_memory; + dnsmasqContext *dctx = dnsmasqContextNew(network->def->name, + DNSMASQ_STATE_DIR); + if (dctx == NULL) { + virReportOOMError(); + goto cleanup; + } if (networkSaveDnsmasqHostsfile(network, dctx, false)) { - if (virAsprintf(&hostsfileArg, "--dhcp-hostsfile=%s", dctx->hostsfile->path) < 0) { - dnsmasqContextFree(dctx); - goto no_memory; - } - APPEND_ARG_LIT(*argv, i++, hostsfileArg); + virCommandAddArgPair(cmd, "--dhcp-hostsfile", + dctx->hostsfile->path); } - dnsmasqContextFree(dctx); } if (network->def->tftproot) { - APPEND_ARG(*argv, i++, "--enable-tftp"); - APPEND_ARG(*argv, i++, "--tftp-root"); - APPEND_ARG(*argv, i++, network->def->tftproot); + virCommandAddArg(cmd, "--enable-tftp"); + virCommandAddArg(cmd, "--tftp-root"); + virCommandAddArg(cmd, network->def->tftproot); } if (network->def->bootfile) { - char *ipaddr = NULL; + + virCommandAddArg(cmd, "--dhcp-boot"); if (VIR_SOCKET_HAS_ADDR(&network->def->bootserver)) { - if (!(ipaddr = virSocketFormatAddr(&network->def->bootserver))) - goto error; - } - char *boot; - int rc = virAsprintf(&boot, "%s%s%s", - network->def->bootfile, - ipaddr ? ",," : "", - ipaddr ? ipaddr : ""); - VIR_FREE(ipaddr); - if (rc < 0) - goto no_memory; + char *bootserver = virSocketFormatAddr(&network->def->bootserver); - APPEND_ARG(*argv, i++, "--dhcp-boot"); - APPEND_ARG_LIT(*argv, i++, boot); + if (!bootserver) + goto cleanup; + virCommandAddArgFormat(cmd, "%s%s%s", + network->def->bootfile, ",,", bootserver); + VIR_FREE(bootserver); + } else { + virCommandAddArg(cmd, network->def->bootfile); + } } -#undef APPEND_ARG - - return 0; - - no_memory: - virReportOOMError(); - error: - if (*argv) { - for (i = 0; (*argv)[i]; i++) - VIR_FREE((*argv)[i]); - VIR_FREE(*argv); - } - return -1; + ret = 0; +cleanup: + VIR_FREE(bridgeaddr); + return ret; } static int dhcpStartDhcpDaemon(virNetworkObjPtr network) { - const char **argv; - char *pidfile; - int ret = -1, i, err; + virCommandPtr cmd = NULL; + char *pidfile = NULL; + int ret = -1, err; network->dnsmasqPid = -1; if (!VIR_SOCKET_IS_FAMILY(&network->def->ipAddress, AF_INET)) { networkReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot start dhcp daemon without IPv4 address for server")); - return -1; + goto cleanup; } if ((err = virFileMakePath(NETWORK_PID_DIR)) != 0) { virReportSystemError(err, _("cannot create directory %s"), NETWORK_PID_DIR); - return -1; + goto cleanup; } if ((err = virFileMakePath(NETWORK_STATE_DIR)) != 0) { virReportSystemError(err, _("cannot create directory %s"), NETWORK_STATE_DIR); - return -1; + goto cleanup; } if (!(pidfile = virFilePid(NETWORK_PID_DIR, network->def->name))) { virReportOOMError(); - return -1; + goto cleanup; } - argv = NULL; - if (networkBuildDnsmasqArgv(network, pidfile, &argv) < 0) { + cmd = virCommandNew(DNSMASQ); + if (networkBuildDnsmasqArgv(network, pidfile, cmd) < 0) { VIR_FREE(pidfile); - return -1; + goto cleanup; } - if (virRun(argv, NULL) < 0) + if (virCommandRun(cmd, NULL) < 0) goto cleanup; /* - * There really is no race here - when dnsmasq daemonizes, - * its leader process stays around until its child has - * actually written its pidfile. So by time virRun exits - * it has waitpid'd and guaranteed the proess has started - * and written a pid + * There really is no race here - when dnsmasq daemonizes, its + * leader process stays around until its child has actually + * written its pidfile. So by time virCommandRun exits it has + * waitpid'd and guaranteed the proess has started and written a + * pid */ if (virFileReadPid(NETWORK_PID_DIR, network->def->name, @@ -656,13 +582,9 @@ dhcpStartDhcpDaemon(virNetworkObjPtr network) goto cleanup; ret = 0; - cleanup: VIR_FREE(pidfile); - for (i = 0; argv[i]; i++) - VIR_FREE(argv[i]); - VIR_FREE(argv); - + virCommandFree(cmd); return ret; } -- 1.7.3.2

3 4

[libvirt] [PATCH 0/4] more memory leak patches
by Eric Blake 10 Dec '10

10 Dec '10

More in the ongoing battle to make libvirtd not so leaky. Also related, and still waiting on a review for: virExec (and thus virCommand) have undefined behavior https://www.redhat.com/archives/libvir-list/2010-December/msg00242.html avoid matchpathcon memory leak (although libselinux 2.0.97 finally patched that leak upstream) https://www.redhat.com/archives/libvir-list/2010-December/msg00001.html The leak in qemu_conf covered in patch 3 was introduced with the conversion to virCommand; if you like patch 4, I'd rather combine those into one patch (but patch 3 is a minimal plug). Eric Blake (4): tests: plug memory leaks conf: plug memory leaks qemu: plug memory leak command: ease use with virBuffer src/conf/domain_conf.c | 10 ++++++++++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_conf.c | 42 +++++++++--------------------------------- src/util/command.c | 43 +++++++++++++++++++++++++++++++++++++++++++ src/util/command.h | 17 +++++++++++++++++ tests/qemuxml2argvtest.c | 3 +-- 6 files changed, 82 insertions(+), 35 deletions(-) -- 1.7.3.2

2 11

[libvirt] [PATCH] virExec: avoid undefined behavior
by Eric Blake 10 Dec '10

10 Dec '10

* src/util/util.c (__virExec): Don't use FD_ISSET on out-of-bounds fd. --- Noticed this one by inspection, while investigating https://bugzilla.redhat.com/show_bug.cgi?id=659855 Don't know if it's the root cause of the crash in that bug, though. src/util/util.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 79ca5d3..1b5bc68 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -570,8 +570,7 @@ __virExec(const char *const*argv, i != null && i != childout && i != childerr && - (!keepfd || - !FD_ISSET(i, keepfd))) { + (!keepfd || (i < FD_SETSIZE && !FD_ISSET(i, keepfd)))) { tmpfd = i; VIR_FORCE_CLOSE(tmpfd); } -- 1.7.3.2

2 2

[libvirt] [PATCH] esx: Refactor storage pool type lookup into a function
by Matthias Bolte 10 Dec '10

10 Dec '10

--- src/esx/esx_storage_driver.c | 130 ++++++++++++++++++------------------------ 1 files changed, 56 insertions(+), 74 deletions(-) diff --git a/src/esx/esx_storage_driver.c b/src/esx/esx_storage_driver.c index 83b53fb..9165f7a 100644 --- a/src/esx/esx_storage_driver.c +++ b/src/esx/esx_storage_driver.c @@ -49,6 +49,58 @@ verify(MD5_DIGEST_SIZE == VIR_UUID_BUFLEN); +static int +esxStoragePoolLookupType(esxVI_Context *ctx, const char *poolName, + int *poolType) +{ + int result = -1; + esxVI_String *propertyNameList = NULL; + esxVI_ObjectContent *datastore = NULL; + esxVI_DynamicProperty *dynamicProperty = NULL; + esxVI_DatastoreInfo *datastoreInfo = NULL; + + if (esxVI_String_AppendValueToList(&propertyNameList, "info") < 0 || + esxVI_LookupDatastoreByName(ctx, poolName, propertyNameList, &datastore, + esxVI_Occurrence_RequiredItem) < 0) { + goto cleanup; + } + + for (dynamicProperty = datastore->propSet; dynamicProperty != NULL; + dynamicProperty = dynamicProperty->_next) { + if (STREQ(dynamicProperty->name, "info")) { + if (esxVI_DatastoreInfo_CastFromAnyType(dynamicProperty->val, + &datastoreInfo) < 0) { + goto cleanup; + } + + break; + } + } + + if (esxVI_LocalDatastoreInfo_DynamicCast(datastoreInfo) != NULL) { + *poolType = VIR_STORAGE_POOL_DIR; + } else if (esxVI_NasDatastoreInfo_DynamicCast(datastoreInfo) != NULL) { + *poolType = VIR_STORAGE_POOL_NETFS; + } else if (esxVI_VmfsDatastoreInfo_DynamicCast(datastoreInfo) != NULL) { + *poolType = VIR_STORAGE_POOL_FS; + } else { + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", + _("DatastoreInfo has unexpected type")); + goto cleanup; + } + + result = 0; + + cleanup: + esxVI_String_Free(&propertyNameList); + esxVI_ObjectContent_Free(&datastore); + esxVI_DatastoreInfo_Free(&datastoreInfo); + + return result; +} + + + static virDrvOpenStatus esxStorageOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, @@ -908,10 +960,6 @@ esxStorageVolumeCreateXML(virStoragePoolPtr pool, const char *xmldesc, { virStorageVolPtr volume = NULL; esxPrivate *priv = pool->conn->storagePrivateData; - esxVI_String *propertyNameList = NULL; - esxVI_ObjectContent *datastore = NULL; - esxVI_DynamicProperty *dynamicProperty = NULL; - esxVI_DatastoreInfo *datastoreInfo = NULL; virStoragePoolDef poolDef; virStorageVolDefPtr def = NULL; char *tmp; @@ -938,36 +986,8 @@ esxStorageVolumeCreateXML(virStoragePoolPtr pool, const char *xmldesc, return NULL; } - /* Lookup storage pool type */ - if (esxVI_String_AppendValueToList(&propertyNameList, "info") < 0 || - esxVI_LookupDatastoreByName(priv->primary, pool->name, - propertyNameList, &datastore, - esxVI_Occurrence_RequiredItem) < 0) { - goto cleanup; - } - - for (dynamicProperty = datastore->propSet; dynamicProperty != NULL; - dynamicProperty = dynamicProperty->_next) { - if (STREQ(dynamicProperty->name, "info")) { - if (esxVI_DatastoreInfo_CastFromAnyType(dynamicProperty->val, - &datastoreInfo) < 0) { - goto cleanup; - } - - break; - } - } - - if (esxVI_LocalDatastoreInfo_DynamicCast(datastoreInfo) != NULL) { - poolDef.type = VIR_STORAGE_POOL_DIR; - } else if (esxVI_NasDatastoreInfo_DynamicCast(datastoreInfo) != NULL) { - poolDef.type = VIR_STORAGE_POOL_NETFS; - } else if (esxVI_VmfsDatastoreInfo_DynamicCast(datastoreInfo) != NULL) { - poolDef.type = VIR_STORAGE_POOL_FS; - } else { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", - _("DatastoreInfo has unexpected type")); - goto cleanup; + if (esxStoragePoolLookupType(priv->primary, pool->name, &poolDef.type) < 0) { + return NULL; } /* Parse config */ @@ -1140,9 +1160,6 @@ esxStorageVolumeCreateXML(virStoragePoolPtr pool, const char *xmldesc, virtualDiskSpec->adapterType = NULL; } - esxVI_String_Free(&propertyNameList); - esxVI_ObjectContent_Free(&datastore); - esxVI_DatastoreInfo_Free(&datastoreInfo); virStorageVolDefFree(def); VIR_FREE(unescapedDatastorePath); VIR_FREE(unescapedDirectoryName); @@ -1216,10 +1233,6 @@ static char * esxStorageVolumeDumpXML(virStorageVolPtr volume, unsigned int flags) { esxPrivate *priv = volume->conn->storagePrivateData; - esxVI_String *propertyNameList = NULL; - esxVI_ObjectContent *datastore = NULL; - esxVI_DynamicProperty *dynamicProperty = NULL; - esxVI_DatastoreInfo *datastoreInfo = NULL; virStoragePoolDef pool; char *datastorePath = NULL; esxVI_FileInfo *fileInfo = NULL; @@ -1238,36 +1251,8 @@ esxStorageVolumeDumpXML(virStorageVolPtr volume, unsigned int flags) return NULL; } - /* Lookup storage pool type */ - if (esxVI_String_AppendValueToList(&propertyNameList, "info") < 0 || - esxVI_LookupDatastoreByName(priv->primary, volume->pool, - propertyNameList, &datastore, - esxVI_Occurrence_RequiredItem) < 0) { - goto cleanup; - } - - for (dynamicProperty = datastore->propSet; dynamicProperty != NULL; - dynamicProperty = dynamicProperty->_next) { - if (STREQ(dynamicProperty->name, "info")) { - if (esxVI_DatastoreInfo_CastFromAnyType(dynamicProperty->val, - &datastoreInfo) < 0) { - goto cleanup; - } - - break; - } - } - - if (esxVI_LocalDatastoreInfo_DynamicCast(datastoreInfo) != NULL) { - pool.type = VIR_STORAGE_POOL_DIR; - } else if (esxVI_NasDatastoreInfo_DynamicCast(datastoreInfo) != NULL) { - pool.type = VIR_STORAGE_POOL_NETFS; - } else if (esxVI_VmfsDatastoreInfo_DynamicCast(datastoreInfo) != NULL) { - pool.type = VIR_STORAGE_POOL_FS; - } else { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", - _("DatastoreInfo has unexpected type")); - goto cleanup; + if (esxStoragePoolLookupType(priv->primary, volume->pool, &pool.type) < 0) { + return NULL; } /* Lookup file info */ @@ -1320,9 +1305,6 @@ esxStorageVolumeDumpXML(virStorageVolPtr volume, unsigned int flags) xml = virStorageVolDefFormat(&pool, &def); cleanup: - esxVI_String_Free(&propertyNameList); - esxVI_ObjectContent_Free(&datastore); - esxVI_DatastoreInfo_Free(&datastoreInfo); VIR_FREE(datastorePath); esxVI_FileInfo_Free(&fileInfo); VIR_FREE(def.key); -- 1.7.0.4

2 2

[libvirt] [PATCH] esx: Improve error reporting for failed tasks
by Matthias Bolte 10 Dec '10

10 Dec '10

Instead of just reporting that a task failed get the localized message from the TaskInfo error and include it in the reported error message. Implement minimal deserialization support for the MethodFault type in order to obtain the actual fault type. For example, this changes the reported error message when trying to create a volume with zero size from Could not create volume to Could not create volume: InvalidArgument - A specified parameter was not correct. Not perfect yet, but better than before. --- src/esx/esx_driver.c | 118 +++++++++++++++++++++++++++++----------- src/esx/esx_storage_driver.c | 8 ++- src/esx/esx_vi.c | 53 ++++++++++++++++-- src/esx/esx_vi.h | 3 +- src/esx/esx_vi_generator.input | 8 +++- src/esx/esx_vi_generator.py | 2 + src/esx/esx_vi_types.c | 51 +++++++++++++++++- src/esx/esx_vi_types.h | 25 +++++++++ 8 files changed, 227 insertions(+), 41 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 8ea6219..03a7be3 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -1706,6 +1706,7 @@ esxDomainSuspend(virDomainPtr domain) esxVI_VirtualMachinePowerState powerState; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; if (esxVI_EnsureSession(priv->primary) < 0) { return -1; @@ -1729,12 +1730,14 @@ esxDomainSuspend(virDomainPtr domain) if (esxVI_SuspendVM_Task(priv->primary, virtualMachine->obj, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not suspend domain")); + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("Could not suspend domain: %s"), + taskInfoErrorMessage); goto cleanup; } @@ -1744,6 +1747,7 @@ esxDomainSuspend(virDomainPtr domain) esxVI_ObjectContent_Free(&virtualMachine); esxVI_String_Free(&propertyNameList); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return result; } @@ -1760,6 +1764,7 @@ esxDomainResume(virDomainPtr domain) esxVI_VirtualMachinePowerState powerState; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; if (esxVI_EnsureSession(priv->primary) < 0) { return -1; @@ -1783,12 +1788,14 @@ esxDomainResume(virDomainPtr domain) &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not resume domain")); + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("Could not resume domain: %s"), + taskInfoErrorMessage); goto cleanup; } @@ -1798,6 +1805,7 @@ esxDomainResume(virDomainPtr domain) esxVI_ObjectContent_Free(&virtualMachine); esxVI_String_Free(&propertyNameList); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return result; } @@ -1901,6 +1909,7 @@ esxDomainDestroy(virDomainPtr domain) esxVI_VirtualMachinePowerState powerState; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; if (priv->vCenter != NULL) { ctx = priv->vCenter; @@ -1930,12 +1939,14 @@ esxDomainDestroy(virDomainPtr domain) if (esxVI_PowerOffVM_Task(ctx, virtualMachine->obj, &task) < 0 || esxVI_WaitForTaskCompletion(ctx, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not destroy domain")); + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("Could not destroy domain: %s"), + taskInfoErrorMessage); goto cleanup; } @@ -1946,6 +1957,7 @@ esxDomainDestroy(virDomainPtr domain) esxVI_ObjectContent_Free(&virtualMachine); esxVI_String_Free(&propertyNameList); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return result; } @@ -2028,6 +2040,7 @@ esxDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) esxVI_VirtualMachineConfigSpec *spec = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; if (esxVI_EnsureSession(priv->primary) < 0) { return -1; @@ -2048,13 +2061,15 @@ esxDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Could not set max-memory to %lu kilobytes"), memory); + _("Could not set max-memory to %lu kilobytes: %s"), memory, + taskInfoErrorMessage); goto cleanup; } @@ -2064,6 +2079,7 @@ esxDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) esxVI_ObjectContent_Free(&virtualMachine); esxVI_VirtualMachineConfigSpec_Free(&spec); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return result; } @@ -2079,6 +2095,7 @@ esxDomainSetMemory(virDomainPtr domain, unsigned long memory) esxVI_VirtualMachineConfigSpec *spec = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; if (esxVI_EnsureSession(priv->primary) < 0) { return -1; @@ -2100,13 +2117,15 @@ esxDomainSetMemory(virDomainPtr domain, unsigned long memory) &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Could not set memory to %lu kilobytes"), memory); + _("Could not set memory to %lu kilobytes: %s"), memory, + taskInfoErrorMessage); goto cleanup; } @@ -2116,6 +2135,7 @@ esxDomainSetMemory(virDomainPtr domain, unsigned long memory) esxVI_ObjectContent_Free(&virtualMachine); esxVI_VirtualMachineConfigSpec_Free(&spec); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return result; } @@ -2394,6 +2414,7 @@ esxDomainSetVcpusFlags(virDomainPtr domain, unsigned int nvcpus, esxVI_VirtualMachineConfigSpec *spec = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; if (flags != VIR_DOMAIN_VCPU_LIVE) { ESX_ERROR(VIR_ERR_INVALID_ARG, _("unsupported flags: (0x%x)"), flags); @@ -2438,13 +2459,15 @@ esxDomainSetVcpusFlags(virDomainPtr domain, unsigned int nvcpus, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Could not set number of virtual CPUs to %d"), nvcpus); + _("Could not set number of virtual CPUs to %d: %s"), nvcpus, + taskInfoErrorMessage); goto cleanup; } @@ -2454,6 +2477,7 @@ esxDomainSetVcpusFlags(virDomainPtr domain, unsigned int nvcpus, esxVI_ObjectContent_Free(&virtualMachine); esxVI_VirtualMachineConfigSpec_Free(&spec); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return result; } @@ -2828,6 +2852,7 @@ esxDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) int id = -1; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; virCheckFlags(0, -1); @@ -2855,12 +2880,14 @@ esxDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not start domain")); + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("Could not start domain: %s"), + taskInfoErrorMessage); goto cleanup; } @@ -2871,6 +2898,7 @@ esxDomainCreateWithFlags(virDomainPtr domain, unsigned int flags) esxVI_ObjectContent_Free(&virtualMachine); esxVI_String_Free(&propertyNameList); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return result; } @@ -2907,6 +2935,7 @@ esxDomainDefineXML(virConnectPtr conn, const char *xml) esxVI_ManagedObjectReference *resourcePool = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; virDomainPtr domain = NULL; if (esxVI_EnsureSession(priv->primary) < 0) { @@ -3066,12 +3095,14 @@ esxDomainDefineXML(virConnectPtr conn, const char *xml) &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, def->uuid, esxVI_Occurrence_OptionalItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not define domain")); + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("Could not define domain: %s"), + taskInfoErrorMessage); goto cleanup; } @@ -3100,6 +3131,7 @@ esxDomainDefineXML(virConnectPtr conn, const char *xml) esxVI_ObjectContent_Free(&hostSystem); esxVI_ManagedObjectReference_Free(&resourcePool); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return domain; } @@ -3340,6 +3372,7 @@ esxDomainSetSchedulerParameters(virDomainPtr domain, esxVI_SharesInfo *sharesInfo = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; int i; if (esxVI_EnsureSession(priv->primary) < 0) { @@ -3434,13 +3467,15 @@ esxDomainSetSchedulerParameters(virDomainPtr domain, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not change scheduler parameters")); + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Could not change scheduler parameters: %s"), + taskInfoErrorMessage); goto cleanup; } @@ -3450,6 +3485,7 @@ esxDomainSetSchedulerParameters(virDomainPtr domain, esxVI_ObjectContent_Free(&virtualMachine); esxVI_VirtualMachineConfigSpec_Free(&spec); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return result; } @@ -3504,6 +3540,7 @@ esxDomainMigratePerform(virDomainPtr domain, esxVI_Event *eventList = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; if (priv->vCenter == NULL) { ESX_ERROR(VIR_ERR_INVALID_ARG, "%s", @@ -3601,14 +3638,16 @@ esxDomainMigratePerform(virDomainPtr domain, &task) < 0 || esxVI_WaitForTaskCompletion(priv->vCenter, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("Could not migrate domain, migration task finished with " - "an error")); + "an error: %s"), + taskInfoErrorMessage); goto cleanup; } @@ -3619,6 +3658,7 @@ esxDomainMigratePerform(virDomainPtr domain, esxVI_ObjectContent_Free(&virtualMachine); esxVI_Event_Free(&eventList); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return result; } @@ -3783,6 +3823,7 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, esxVI_VirtualMachineSnapshotTree *snapshotTreeParent = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; virDomainSnapshotPtr snapshot = NULL; virCheckFlags(0, NULL); @@ -3820,12 +3861,14 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, esxVI_Boolean_False, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not create snapshot")); + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("Could not create snapshot: %s"), + taskInfoErrorMessage); goto cleanup; } @@ -3836,6 +3879,7 @@ esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, esxVI_ObjectContent_Free(&virtualMachine); esxVI_VirtualMachineSnapshotTree_Free(&rootSnapshotList); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return snapshot; } @@ -4058,6 +4102,7 @@ esxDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, unsigned int flags) esxVI_VirtualMachineSnapshotTree *snapshotTreeParent = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; virCheckFlags(0, -1); @@ -4077,13 +4122,15 @@ esxDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, unsigned int flags) &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, snapshot->domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Could not revert to snapshot '%s'"), snapshot->name); + _("Could not revert to snapshot '%s': %s"), snapshot->name, + taskInfoErrorMessage); goto cleanup; } @@ -4092,6 +4139,7 @@ esxDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, unsigned int flags) cleanup: esxVI_VirtualMachineSnapshotTree_Free(&rootSnapshotList); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return result; } @@ -4109,6 +4157,7 @@ esxDomainSnapshotDelete(virDomainSnapshotPtr snapshot, unsigned int flags) esxVI_Boolean removeChildren = esxVI_Boolean_False; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN, -1); @@ -4132,13 +4181,15 @@ esxDomainSnapshotDelete(virDomainSnapshotPtr snapshot, unsigned int flags) removeChildren, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, snapshot->domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Could not delete snapshot '%s'"), snapshot->name); + _("Could not delete snapshot '%s': %s"), snapshot->name, + taskInfoErrorMessage); goto cleanup; } @@ -4147,6 +4198,7 @@ esxDomainSnapshotDelete(virDomainSnapshotPtr snapshot, unsigned int flags) cleanup: esxVI_VirtualMachineSnapshotTree_Free(&rootSnapshotList); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return result; } @@ -4163,6 +4215,7 @@ esxDomainSetMemoryParameters(virDomainPtr domain, virMemoryParameterPtr params, esxVI_VirtualMachineConfigSpec *spec = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; int i; virCheckFlags(0, -1); @@ -4199,13 +4252,15 @@ esxDomainSetMemoryParameters(virDomainPtr domain, virMemoryParameterPtr params, &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, domain->uuid, esxVI_Occurrence_RequiredItem, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not change memory parameters")); + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, + _("Could not change memory parameters: %s"), + taskInfoErrorMessage); goto cleanup; } @@ -4215,6 +4270,7 @@ esxDomainSetMemoryParameters(virDomainPtr domain, virMemoryParameterPtr params, esxVI_ObjectContent_Free(&virtualMachine); esxVI_VirtualMachineConfigSpec_Free(&spec); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); return result; } diff --git a/src/esx/esx_storage_driver.c b/src/esx/esx_storage_driver.c index 72e0d7a..83b53fb 100644 --- a/src/esx/esx_storage_driver.c +++ b/src/esx/esx_storage_driver.c @@ -926,6 +926,7 @@ esxStorageVolumeCreateXML(virStoragePoolPtr pool, const char *xmldesc, esxVI_FileBackedVirtualDiskSpec *virtualDiskSpec = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; char *uuid_string = NULL; char *key = NULL; @@ -1092,12 +1093,14 @@ esxStorageVolumeCreateXML(virStoragePoolPtr pool, const char *xmldesc, esxVI_VirtualDiskSpec_DynamicCast(virtualDiskSpec), &task) < 0 || esxVI_WaitForTaskCompletion(priv->primary, task, NULL, esxVI_Occurrence_None, - priv->autoAnswer, &taskInfoState) < 0) { + priv->autoAnswer, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { - ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not create volume")); + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, _("Could not create volume: %s"), + taskInfoErrorMessage); goto cleanup; } @@ -1151,6 +1154,7 @@ esxStorageVolumeCreateXML(virStoragePoolPtr pool, const char *xmldesc, esxVI_FileInfo_Free(&fileInfo); esxVI_FileBackedVirtualDiskSpec_Free(&virtualDiskSpec); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); VIR_FREE(uuid_string); VIR_FREE(key); diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 9c96650..74a2a42 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -3009,6 +3009,7 @@ esxVI_LookupFileInfoByDatastorePath(esxVI_Context *ctx, esxVI_FloppyImageFileQuery *floppyImageFileQuery = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; esxVI_TaskInfo *taskInfo = NULL; esxVI_HostDatastoreBrowserSearchResults *searchResults = NULL; @@ -3128,13 +3129,15 @@ esxVI_LookupFileInfoByDatastorePath(esxVI_Context *ctx, datastorePathWithoutFileName, searchSpec, &task) < 0 || esxVI_WaitForTaskCompletion(ctx, task, NULL, esxVI_Occurrence_None, - esxVI_Boolean_False, &taskInfoState) < 0) { + esxVI_Boolean_False, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Could not serach in datastore '%s'"), datastoreName); + _("Could not search in datastore '%s': %s"), + datastoreName, taskInfoErrorMessage); goto cleanup; } @@ -3179,6 +3182,7 @@ esxVI_LookupFileInfoByDatastorePath(esxVI_Context *ctx, esxVI_ManagedObjectReference_Free(&hostDatastoreBrowser); esxVI_HostDatastoreBrowserSearchSpec_Free(&searchSpec); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); esxVI_TaskInfo_Free(&taskInfo); esxVI_HostDatastoreBrowserSearchResults_Free(&searchResults); @@ -3203,6 +3207,7 @@ esxVI_LookupDatastoreContentByDatastoreName char *datastorePath = NULL; esxVI_ManagedObjectReference *task = NULL; esxVI_TaskInfoState taskInfoState; + char *taskInfoErrorMessage = NULL; esxVI_TaskInfo *taskInfo = NULL; if (searchResultsList == NULL || *searchResultsList != NULL) { @@ -3269,13 +3274,15 @@ esxVI_LookupDatastoreContentByDatastoreName datastorePath, searchSpec, &task) < 0 || esxVI_WaitForTaskCompletion(ctx, task, NULL, esxVI_Occurrence_None, - esxVI_Boolean_False, &taskInfoState) < 0) { + esxVI_Boolean_False, &taskInfoState, + &taskInfoErrorMessage) < 0) { goto cleanup; } if (taskInfoState != esxVI_TaskInfoState_Success) { ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Could not serach in datastore '%s'"), datastoreName); + _("Could not serach in datastore '%s': %s"), + datastoreName, taskInfoErrorMessage); goto cleanup; } @@ -3294,6 +3301,7 @@ esxVI_LookupDatastoreContentByDatastoreName esxVI_HostDatastoreBrowserSearchSpec_Free(&searchSpec); VIR_FREE(datastorePath); esxVI_ManagedObjectReference_Free(&task); + VIR_FREE(taskInfoErrorMessage); esxVI_TaskInfo_Free(&taskInfo); return result; @@ -3472,7 +3480,8 @@ esxVI_WaitForTaskCompletion(esxVI_Context *ctx, const unsigned char *virtualMachineUuid, esxVI_Occurrence virtualMachineOccurrence, esxVI_Boolean autoAnswer, - esxVI_TaskInfoState *finalState) + esxVI_TaskInfoState *finalState, + char **errorMessage) { int result = -1; esxVI_ObjectSpec *objectSpec = NULL; @@ -3489,6 +3498,11 @@ esxVI_WaitForTaskCompletion(esxVI_Context *ctx, esxVI_Boolean blocked = esxVI_Boolean_Undefined; esxVI_TaskInfo *taskInfo = NULL; + if (errorMessage == NULL || *errorMessage != NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); + return -1; + } + version = strdup(""); if (version == NULL) { @@ -3609,6 +3623,35 @@ esxVI_WaitForTaskCompletion(esxVI_Context *ctx, goto cleanup; } + if (*finalState != esxVI_TaskInfoState_Success) { + if (esxVI_LookupTaskInfoByTask(ctx, task, &taskInfo)) { + goto cleanup; + } + + if (taskInfo->error == NULL) { + *errorMessage = strdup(_("Unknown error")); + + if (*errorMessage == NULL) { + virReportOOMError(); + goto cleanup; + } + } else if (taskInfo->error->localizedMessage == NULL) { + *errorMessage = strdup(taskInfo->error->fault->_actualType); + + if (*errorMessage == NULL) { + virReportOOMError(); + goto cleanup; + } + } else { + if (virAsprintf(errorMessage, "%s - %s", + taskInfo->error->fault->_actualType, + taskInfo->error->localizedMessage) < 0) { + virReportOOMError(); + goto cleanup; + } + } + } + result = 0; cleanup: diff --git a/src/esx/esx_vi.h b/src/esx/esx_vi.h index 26779ad..20e6fb5 100644 --- a/src/esx/esx_vi.h +++ b/src/esx/esx_vi.h @@ -429,7 +429,8 @@ int esxVI_WaitForTaskCompletion(esxVI_Context *ctx, const unsigned char *virtualMachineUuid, esxVI_Occurrence virtualMachineOccurrence, esxVI_Boolean autoAnswer, - esxVI_TaskInfoState *finalState); + esxVI_TaskInfoState *finalState, + char **errorMessage); int esxVI_ParseHostCpuIdInfo(esxVI_ParsedHostCpuIdInfo *parsedHostCpuIdInfo, esxVI_HostCpuIdInfo *hostCpuIdInfo); diff --git a/src/esx/esx_vi_generator.input b/src/esx/esx_vi_generator.input index 2ee513a..991ce8a 100644 --- a/src/esx/esx_vi_generator.input +++ b/src/esx/esx_vi_generator.input @@ -311,6 +311,12 @@ object LocalDatastoreInfo extends DatastoreInfo end +object LocalizedMethodFault + MethodFault fault r + String localizedMessage o +end + + object NasDatastoreInfo extends DatastoreInfo HostNasVolume nas o end @@ -494,7 +500,7 @@ object TaskInfo TaskInfoState state r Boolean cancelled r Boolean cancelable r - LocalizedMethodFault error i + LocalizedMethodFault error o AnyType result o Int progress o TaskReason reason i diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index 4593379..4a8a9dc 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -1139,6 +1139,7 @@ predefined_objects = ["AnyType", "Long", "String", "DateTime", + "MethodFault", "ManagedObjectReference"] @@ -1172,6 +1173,7 @@ additional_object_features = { "DatastoreHostMount" : Object.FEATURE__DE removed_object_features = { "DynamicProperty" : Object.FEATURE__SERIALIZE, + "LocalizedMethodFault" : Object.FEATURE__SERIALIZE, "ObjectContent" : Object.FEATURE__SERIALIZE, "ObjectUpdate" : Object.FEATURE__SERIALIZE, "PropertyChange" : Object.FEATURE__SERIALIZE, diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c index ad45483..a70e1e0 100644 --- a/src/esx/esx_vi_types.c +++ b/src/esx/esx_vi_types.c @@ -701,6 +701,9 @@ esxVI_Type_ToString(esxVI_Type type) case esxVI_Type_Fault: return "Fault"; + case esxVI_Type_MethodFault: + return "MethodFault"; + case esxVI_Type_ManagedObjectReference: return "ManagedObjectReference"; @@ -741,6 +744,8 @@ esxVI_Type_FromString(const char *type) return esxVI_Type_DateTime; } else if (STREQ(type, "Fault")) { return esxVI_Type_Fault; + } else if (STREQ(type, "MethodFault")) { + return esxVI_Type_MethodFault; } else if (STREQ(type, "ManagedObjectReference")) { return esxVI_Type_ManagedObjectReference; } else if (STREQ(type, "Datacenter")) { @@ -751,7 +756,6 @@ esxVI_Type_FromString(const char *type) return esxVI_Type_HostSystem; } - #include "esx_vi_types.generated.typefromstring" else { @@ -1447,6 +1451,51 @@ ESX_VI__TEMPLATE__DESERIALIZE(Fault, /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + * VI Type: MethodFault + */ + +/* esxVI_MethodFault_Alloc */ +ESX_VI__TEMPLATE__ALLOC(MethodFault); + +/* esxVI_MethodFault_Free */ +ESX_VI__TEMPLATE__FREE(MethodFault, +{ + VIR_FREE(item->_actualType); +}) + +int +esxVI_MethodFault_Deserialize(xmlNodePtr node, esxVI_MethodFault **methodFault) +{ + if (methodFault == NULL || *methodFault != NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid argument")); + return -1; + } + + if (esxVI_MethodFault_Alloc(methodFault) < 0) { + return -1; + } + + (*methodFault)->_actualType = + (char *)xmlGetNsProp(node, BAD_CAST "type", + BAD_CAST "http://www.w3.org/2001/XMLSchema-instance"); + + if ((*methodFault)->_actualType == NULL) { + ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", + _("MethodFault is missing 'type' property")); + goto failure; + } + + return 0; + + failure: + esxVI_MethodFault_Free(methodFault); + + return -1; +} + + + +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * VI Type: ManagedObjectReference */ diff --git a/src/esx/esx_vi_types.h b/src/esx/esx_vi_types.h index 901eda0..64bf2dc 100644 --- a/src/esx/esx_vi_types.h +++ b/src/esx/esx_vi_types.h @@ -48,6 +48,7 @@ typedef struct _esxVI_DateTime esxVI_DateTime; */ typedef struct _esxVI_Fault esxVI_Fault; +typedef struct _esxVI_MethodFault esxVI_MethodFault; typedef struct _esxVI_ManagedObjectReference esxVI_ManagedObjectReference; typedef struct _esxVI_Datacenter esxVI_Datacenter; typedef struct _esxVI_ComputeResource esxVI_ComputeResource; @@ -71,6 +72,7 @@ enum _esxVI_Type { esxVI_Type_Long, esxVI_Type_DateTime, esxVI_Type_Fault, + esxVI_Type_MethodFault, esxVI_Type_ManagedObjectReference, esxVI_Type_Datacenter, esxVI_Type_ComputeResource, @@ -279,6 +281,29 @@ int esxVI_Fault_Deserialize(xmlNodePtr node, esxVI_Fault **fault); /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * + * VI Type: MethodFault + */ + +/* + * FIXME: This is just a minimal implementation of the MethodFault type. + * A complete implementation would require to implement dozens of + * extending types too. + */ +struct _esxVI_MethodFault { + esxVI_MethodFault *_unused; /* optional */ + esxVI_Type _type; /* required */ + + char *_actualType; /* required */ +}; + +int esxVI_MethodFault_Alloc(esxVI_MethodFault **methodfault); +void esxVI_MethodFault_Free(esxVI_MethodFault **methodFault); +int esxVI_MethodFault_Deserialize(xmlNodePtr node, + esxVI_MethodFault **methodFault); + + + +/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * VI Type: ManagedObjectReference */ -- 1.7.0.4

2 2