- Devel - libvirt List Archives

[libvirt] [PATCH v2] virObject: Error on suspicious ref and unref
by Michal Privoznik 29 Nov '13

29 Nov '13

https://bugzilla.redhat.com/show_bug.cgi?id=1033061 Since our transformation into virObject is not complete and we must do ref and unref ourselves there's a chance that we will get it wrong. That is, while one thread is doing unref and subsequent dispose another thread may come and do the ref & unref on stale pointer. This results in dispose being called twice (and possibly simultaneously). These kind of errors are hard to catch so we should at least throw an error into logs if such situation occurs. In fact, I've seen a stack trace showing this error had happen (obj = 0x7f4968018260): Thread 2 (Thread 0x7f498596b880 (LWP 13394)): [...] #4 0x00007f496ff44607 in ncf_close (ncf=0x7f496801e3d0) at netcf.c:101 __PRETTY_FUNCTION__ = "ncf_close" #5 0x00007f4984f3f31b in virObjectUnref (anyobj=<optimized out>) at util/virobject.c:262 klass = 0x7f4968019020 obj = 0x7f4968018260 lastRef = true __func__ = "virObjectUnref" #6 0x00007f4970159785 in netcfStateCleanup () at interface/interface_backend_netcf.c:109 No locals. #7 0x00007f4984fc0e28 in virStateCleanup () at libvirt.c:883 i = 6 ret = 0 #8 0x00007f49859b55fd in main (argc=<optimized out>, argv=<optimized out>) at libvirtd.c:1549 srv = 0x7f498696ed00 remote_config_file = 0x0 statuswrite = -1 ret = <optimized out> pid_file_fd = 5 pid_file = 0x0 sock_file = 0x0 sock_file_ro = 0x0 timeout = -1 verbose = 0 godaemon = 0 ipsock = 0 config = 0x7f498696e760 privileged = <optimized out> implicit_conf = <optimized out> run_dir = 0x0 old_umask = <optimized out> opts = {{name = 0x7f49859e2b53 "verbose", has_arg = 0, flag = 0x7fff45057808, val = 118}, {name = 0x7f49859e2b5b "daemon", has_arg = 0, flag = 0x7fff4505780c, val = 100}, {name = 0x7f49859e2b62 "listen", has_arg = 0, flag = 0x7fff45057810, val = 108}, {name = 0x7f49859e2c9f "config", has_arg = 1, flag = 0x0, val = 102}, {name = 0x7f49859e2c00 "timeout", has_arg = 1, flag = 0x0, val = 116}, {name = 0x7f49859e2b69 "pid-file", has_arg = 1, flag = 0x0, val = 112}, {name = 0x7f49859e2b72 "version", has_arg = 0, flag = 0x0, val = 86}, {name = 0x7f49859e2b7a "help", has_arg = 0, flag = 0x0, val = 104}, {name = 0x0, has_arg = 0, flag = 0x0, val = 0}} __func__ = "main" Thread 1 (Thread 0x7f496d6a4700 (LWP 13405)): [...] #6 0x00007f496ff44607 in ncf_close (ncf=0x7f496801e3d0) at netcf.c:101 __PRETTY_FUNCTION__ = "ncf_close" #7 0x00007f4984f3f31b in virObjectUnref (anyobj=<optimized out>) at util/virobject.c:262 klass = 0x7f4968019020 obj = 0x7f4968018260 lastRef = true __func__ = "virObjectUnref" #8 0x00007f4970157752 in netcfInterfaceClose (conn=0x7f49680a3260) at interface/interface_backend_netcf.c:265 No locals. #9 0x00007f4984fb7984 in virConnectDispose (obj=0x7f49680a3260) at datatypes.c:149 conn = 0x7f49680a3260 #10 0x00007f4984f3f31b in virObjectUnref (anyobj=anyobj@entry=0x7f49680a3260) at util/virobject.c:262 klass = 0x7f49681d6760 obj = 0x7f49680a3260 lastRef = true __func__ = "virObjectUnref" #11 0x00007f4984fc11bf in virConnectClose (conn=conn@entry=0x7f49680a3260) at libvirt.c:1532 __func__ = "virConnectClose" __FUNCTION__ = "virConnectClose" #12 0x00007f496eced281 in virLXCProcessAutostartAll (driver=0x7f49681ffaa0) at lxc/lxc_process.c:1426 conn = 0x7f49680a3260 data = {driver = 0x7f49681ffaa0, conn = 0x7f49680a3260} #13 0x00007f4984fc0d21 in virStateInitialize (privileged=true, callback=callback@entry=0x7f49859b7180 <daemonInhibitCallback>, opaque=opaque@entry=0x7f498696ed00) at libvirt.c:864 i = 8 __func__ = "virStateInitialize" #14 0x00007f49859b71db in daemonRunStateInit (opaque=opaque@entry=0x7f498696ed00) at libvirtd.c:906 srv = 0x7f498696ed00 sysident = 0x7f4968000ae0 __func__ = "daemonRunStateInit" #15 0x00007f4984f4efe1 in virThreadHelper (data=<optimized out>) at util/virthreadpthread.c:161 args = 0x0 local = {func = 0x7f49859b71a0 <daemonRunStateInit>, opaque = 0x7f498696ed00} #16 0x00007f4982a40de3 in start_thread (arg=0x7f496d6a4700) at pthread_create.c:308 __res = <optimized out> pd = 0x7f496d6a4700 now = <optimized out> unwind_buf = {cancel_jmp_buf = {{jmp_buf = {139953345021696, 7118305506502180663, 0, 139953345022400, 139953345021696, 140734351374104, -7179978120810397897, -7180347023594422473}, mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0}, data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}} not_first_call = <optimized out> pagesize_m1 = <optimized out> sp = <optimized out> freesize = <optimized out> #17 0x00007f498236716d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:113 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- diff to v1: - s/virReportError/VIR_ERROR/ - fix bool = int assignment src/nwfilter/nwfilter_dhcpsnoop.c | 6 ++--- src/util/viratomic.h | 53 +++++++++++++++++++++++++++++++-------- src/util/virobject.c | 13 +++++++--- 3 files changed, 54 insertions(+), 18 deletions(-) diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index e8fcfef..71380bb 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -893,7 +893,7 @@ virNWFilterSnoopReqLeaseDel(virNWFilterSnoopReqPtr req, skip_instantiate: VIR_FREE(ipl); - virAtomicIntDecAndTest(&virNWFilterSnoopState.nLeases); + virAtomicIntDec(&virNWFilterSnoopState.nLeases); lease_not_found: VIR_FREE(ipstr); @@ -1169,7 +1169,7 @@ static void virNWFilterDHCPDecodeWorker(void *jobdata, void *opaque) _("Instantiation of rules failed on " "interface '%s'"), req->ifname); } - virAtomicIntDecAndTest(job->qCtr); + virAtomicIntDec(job->qCtr); VIR_FREE(job); } @@ -1562,7 +1562,7 @@ exit: pcap_close(pcapConf[i].handle); } - virAtomicIntDecAndTest(&virNWFilterSnoopState.nThreads); + virAtomicIntDec(&virNWFilterSnoopState.nThreads); return; } diff --git a/src/util/viratomic.h b/src/util/viratomic.h index 4d7f7e5..48450b9 100644 --- a/src/util/viratomic.h +++ b/src/util/viratomic.h @@ -68,6 +68,18 @@ VIR_STATIC int virAtomicIntInc(volatile int *atomic) ATTRIBUTE_NONNULL(1); /** + * virAtomicIntDec: + * Decrements the value of atomic by 1. + * + * Think of this operation as an atomic version of + * { *atomic -= 1; return *atomic == 0; } + * + * This call acts as a full compiler and hardware memory barrier. + */ +VIR_STATIC int virAtomicIntDec(volatile int *atomic) + ATTRIBUTE_NONNULL(1); + +/** * virAtomicIntDecAndTest: * Decrements the value of atomic by 1. * @@ -75,6 +87,8 @@ VIR_STATIC int virAtomicIntInc(volatile int *atomic) * { *atomic -= 1; return *atomic == 0; } * * This call acts as a full compiler and hardware memory barrier. + * Returns true if the resulting decremented value is zero, + * false otherwise. */ VIR_STATIC bool virAtomicIntDecAndTest(volatile int *atomic) ATTRIBUTE_NONNULL(1); @@ -176,12 +190,15 @@ VIR_STATIC unsigned int virAtomicIntXor(volatile unsigned int *atomic, (void)(0 ? *(atomic) ^ *(atomic) : 0); \ __sync_add_and_fetch((atomic), 1); \ })) +# define virAtomicIntDec(atomic) \ + (__extension__ ({ \ + (void)verify_true(sizeof(*(atomic)) == sizeof(int)); \ + (void)(0 ? *(atomic) ^ *(atomic) : 0); \ + __sync_sub_and_fetch((atomic), 1); \ + })) # define virAtomicIntDecAndTest(atomic) \ - (__extension__ ({ \ - (void)verify_true(sizeof(*(atomic)) == sizeof(int)); \ - (void)(0 ? *(atomic) ^ *(atomic) : 0); \ - __sync_fetch_and_sub((atomic), 1) == 1; \ - })) + (virAtomicIntDec(atomic) == 0) + # define virAtomicIntCompareExchange(atomic, oldval, newval) \ (__extension__ ({ \ (void)verify_true(sizeof(*(atomic)) == sizeof(int)); \ @@ -252,10 +269,16 @@ virAtomicIntInc(volatile int *atomic) return InterlockedIncrement((volatile LONG *)atomic); } +static inline int +virAtomicIntDec(volatile int *atomic) +{ + return InterlockedDecrement((volatile LONG *)atomic); +} + static inline bool virAtomicIntDecAndTest(volatile int *atomic) { - return InterlockedDecrement((volatile LONG *)atomic) == 0; + return virAtomicIntDec(atomic) == 0; } static inline bool @@ -334,16 +357,22 @@ virAtomicIntInc(volatile int *atomic) return value; } -static inline bool -virAtomicIntDecAndTest(volatile int *atomic) +static inline int +virAtomicIntDec(volatile int *atomic) { - bool is_zero; + int value; pthread_mutex_lock(&virAtomicLock); - is_zero = --(*atomic) == 0; + value = --(*atomic); pthread_mutex_unlock(&virAtomicLock); - return is_zero; + return value; +} + +static inline bool +virAtomicIntDecAndTest(volatile int *atomic) +{ + return virAtomicIntDec(atomic) == 0; } static inline bool @@ -436,6 +465,8 @@ virAtomicIntXor(volatile unsigned int *atomic, virAtomicIntSet((int *)atomic, val) # define virAtomicIntInc(atomic) \ virAtomicIntInc((int *)atomic) +# define virAtomicIntDec(atomic) \ + virAtomicIntDec((int *)atomic) # define virAtomicIntDecAndTest(atomic) \ virAtomicIntDecAndTest((int *)atomic) # define virAtomicIntCompareExchange(atomic, oldval, newval) \ diff --git a/src/util/virobject.c b/src/util/virobject.c index 61b5413..a398ab5 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -252,9 +252,9 @@ bool virObjectUnref(void *anyobj) if (!obj) return false; - bool lastRef = virAtomicIntDecAndTest(&obj->refs); + int newRefs = virAtomicIntDec(&obj->refs); PROBE(OBJECT_UNREF, "obj=%p", obj); - if (lastRef) { + if (newRefs == 0) { PROBE(OBJECT_DISPOSE, "obj=%p", obj); virClassPtr klass = obj->klass; while (klass) { @@ -268,9 +268,11 @@ bool virObjectUnref(void *anyobj) obj->magic = 0xDEADBEEF; obj->klass = (void*)0xDEADBEEF; VIR_FREE(obj); + } else if (newRefs < 0) { + VIR_ERROR(_("Suspicious unrefing of object %p"), anyobj); } - return !lastRef; + return newRefs > 0; } @@ -289,8 +291,11 @@ void *virObjectRef(void *anyobj) if (!obj) return NULL; - virAtomicIntInc(&obj->refs); + bool firstRef = virAtomicIntInc(&obj->refs) <= 1; PROBE(OBJECT_REF, "obj=%p", obj); + if (firstRef) { + VIR_ERROR(_("Suspicious refing of object %p"), anyobj); + } return anyobj; } -- 1.8.4.4

1 1

[libvirt] [PATCH] virObject: Error on suspicious ref and unref
by Michal Privoznik 28 Nov '13

28 Nov '13

https://bugzilla.redhat.com/show_bug.cgi?id=1033061 Since our transformation into virObject is not complete and we must do ref and unref ourselves there's a chance that we will get it wrong. That is, while one thread is doing unref and subsequent dispose another thread may come and do the ref & unref on stale pointer. This results in dispose being called twice (and possibly simultaneously). These kind of errors are hard to catch so we should at least throw an error into logs if such situation occurs. In fact, I've seen a stack trace showing this error had happen (obj = 0x7f4968018260): Thread 2 (Thread 0x7f498596b880 (LWP 13394)): [...] #4 0x00007f496ff44607 in ncf_close (ncf=0x7f496801e3d0) at netcf.c:101 __PRETTY_FUNCTION__ = "ncf_close" #5 0x00007f4984f3f31b in virObjectUnref (anyobj=<optimized out>) at util/virobject.c:262 klass = 0x7f4968019020 obj = 0x7f4968018260 lastRef = true __func__ = "virObjectUnref" #6 0x00007f4970159785 in netcfStateCleanup () at interface/interface_backend_netcf.c:109 No locals. #7 0x00007f4984fc0e28 in virStateCleanup () at libvirt.c:883 i = 6 ret = 0 #8 0x00007f49859b55fd in main (argc=<optimized out>, argv=<optimized out>) at libvirtd.c:1549 srv = 0x7f498696ed00 remote_config_file = 0x0 statuswrite = -1 ret = <optimized out> pid_file_fd = 5 pid_file = 0x0 sock_file = 0x0 sock_file_ro = 0x0 timeout = -1 verbose = 0 godaemon = 0 ipsock = 0 config = 0x7f498696e760 privileged = <optimized out> implicit_conf = <optimized out> run_dir = 0x0 old_umask = <optimized out> opts = {{name = 0x7f49859e2b53 "verbose", has_arg = 0, flag = 0x7fff45057808, val = 118}, {name = 0x7f49859e2b5b "daemon", has_arg = 0, flag = 0x7fff4505780c, val = 100}, {name = 0x7f49859e2b62 "listen", has_arg = 0, flag = 0x7fff45057810, val = 108}, {name = 0x7f49859e2c9f "config", has_arg = 1, flag = 0x0, val = 102}, {name = 0x7f49859e2c00 "timeout", has_arg = 1, flag = 0x0, val = 116}, {name = 0x7f49859e2b69 "pid-file", has_arg = 1, flag = 0x0, val = 112}, {name = 0x7f49859e2b72 "version", has_arg = 0, flag = 0x0, val = 86}, {name = 0x7f49859e2b7a "help", has_arg = 0, flag = 0x0, val = 104}, {name = 0x0, has_arg = 0, flag = 0x0, val = 0}} __func__ = "main" Thread 1 (Thread 0x7f496d6a4700 (LWP 13405)): [...] #6 0x00007f496ff44607 in ncf_close (ncf=0x7f496801e3d0) at netcf.c:101 __PRETTY_FUNCTION__ = "ncf_close" #7 0x00007f4984f3f31b in virObjectUnref (anyobj=<optimized out>) at util/virobject.c:262 klass = 0x7f4968019020 obj = 0x7f4968018260 lastRef = true __func__ = "virObjectUnref" #8 0x00007f4970157752 in netcfInterfaceClose (conn=0x7f49680a3260) at interface/interface_backend_netcf.c:265 No locals. #9 0x00007f4984fb7984 in virConnectDispose (obj=0x7f49680a3260) at datatypes.c:149 conn = 0x7f49680a3260 #10 0x00007f4984f3f31b in virObjectUnref (anyobj=anyobj@entry=0x7f49680a3260) at util/virobject.c:262 klass = 0x7f49681d6760 obj = 0x7f49680a3260 lastRef = true __func__ = "virObjectUnref" #11 0x00007f4984fc11bf in virConnectClose (conn=conn@entry=0x7f49680a3260) at libvirt.c:1532 __func__ = "virConnectClose" __FUNCTION__ = "virConnectClose" #12 0x00007f496eced281 in virLXCProcessAutostartAll (driver=0x7f49681ffaa0) at lxc/lxc_process.c:1426 conn = 0x7f49680a3260 data = {driver = 0x7f49681ffaa0, conn = 0x7f49680a3260} #13 0x00007f4984fc0d21 in virStateInitialize (privileged=true, callback=callback@entry=0x7f49859b7180 <daemonInhibitCallback>, opaque=opaque@entry=0x7f498696ed00) at libvirt.c:864 i = 8 __func__ = "virStateInitialize" #14 0x00007f49859b71db in daemonRunStateInit (opaque=opaque@entry=0x7f498696ed00) at libvirtd.c:906 srv = 0x7f498696ed00 sysident = 0x7f4968000ae0 __func__ = "daemonRunStateInit" #15 0x00007f4984f4efe1 in virThreadHelper (data=<optimized out>) at util/virthreadpthread.c:161 args = 0x0 local = {func = 0x7f49859b71a0 <daemonRunStateInit>, opaque = 0x7f498696ed00} #16 0x00007f4982a40de3 in start_thread (arg=0x7f496d6a4700) at pthread_create.c:308 __res = <optimized out> pd = 0x7f496d6a4700 now = <optimized out> unwind_buf = {cancel_jmp_buf = {{jmp_buf = {139953345021696, 7118305506502180663, 0, 139953345022400, 139953345021696, 140734351374104, -7179978120810397897, -7180347023594422473}, mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0}, data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}} not_first_call = <optimized out> pagesize_m1 = <optimized out> sp = <optimized out> freesize = <optimized out> #17 0x00007f498236716d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:113 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/nwfilter/nwfilter_dhcpsnoop.c | 6 ++--- src/util/viratomic.h | 53 +++++++++++++++++++++++++++++++-------- src/util/virobject.c | 17 ++++++++++--- 3 files changed, 58 insertions(+), 18 deletions(-) diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index e8fcfef..71380bb 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -893,7 +893,7 @@ virNWFilterSnoopReqLeaseDel(virNWFilterSnoopReqPtr req, skip_instantiate: VIR_FREE(ipl); - virAtomicIntDecAndTest(&virNWFilterSnoopState.nLeases); + virAtomicIntDec(&virNWFilterSnoopState.nLeases); lease_not_found: VIR_FREE(ipstr); @@ -1169,7 +1169,7 @@ static void virNWFilterDHCPDecodeWorker(void *jobdata, void *opaque) _("Instantiation of rules failed on " "interface '%s'"), req->ifname); } - virAtomicIntDecAndTest(job->qCtr); + virAtomicIntDec(job->qCtr); VIR_FREE(job); } @@ -1562,7 +1562,7 @@ exit: pcap_close(pcapConf[i].handle); } - virAtomicIntDecAndTest(&virNWFilterSnoopState.nThreads); + virAtomicIntDec(&virNWFilterSnoopState.nThreads); return; } diff --git a/src/util/viratomic.h b/src/util/viratomic.h index 4d7f7e5..877900e 100644 --- a/src/util/viratomic.h +++ b/src/util/viratomic.h @@ -68,6 +68,18 @@ VIR_STATIC int virAtomicIntInc(volatile int *atomic) ATTRIBUTE_NONNULL(1); /** + * virAtomicIntDec: + * Decrements the value of atomic by 1. + * + * Think of this operation as an atomic version of + * { *atomic -= 1; return *atomic == 0; } + * + * This call acts as a full compiler and hardware memory barrier. + */ +VIR_STATIC int virAtomicIntDec(volatile int *atomic) + ATTRIBUTE_NONNULL(1); + +/** * virAtomicIntDecAndTest: * Decrements the value of atomic by 1. * @@ -75,6 +87,8 @@ VIR_STATIC int virAtomicIntInc(volatile int *atomic) * { *atomic -= 1; return *atomic == 0; } * * This call acts as a full compiler and hardware memory barrier. + * Returns true if the resulting decremented value is zero, + * false otherwise. */ VIR_STATIC bool virAtomicIntDecAndTest(volatile int *atomic) ATTRIBUTE_NONNULL(1); @@ -176,12 +190,15 @@ VIR_STATIC unsigned int virAtomicIntXor(volatile unsigned int *atomic, (void)(0 ? *(atomic) ^ *(atomic) : 0); \ __sync_add_and_fetch((atomic), 1); \ })) +# define virAtomicIntDec(atomic) \ + (__extension__ ({ \ + (void)verify_true(sizeof(*(atomic)) == sizeof(int)); \ + (void)(0 ? *(atomic) ^ *(atomic) : 0); \ + __sync_sub_and_fetch((atomic), 1); \ + })) # define virAtomicIntDecAndTest(atomic) \ - (__extension__ ({ \ - (void)verify_true(sizeof(*(atomic)) == sizeof(int)); \ - (void)(0 ? *(atomic) ^ *(atomic) : 0); \ - __sync_fetch_and_sub((atomic), 1) == 1; \ - })) + (virAtomicIntDec(atomic) == 0) + # define virAtomicIntCompareExchange(atomic, oldval, newval) \ (__extension__ ({ \ (void)verify_true(sizeof(*(atomic)) == sizeof(int)); \ @@ -252,10 +269,16 @@ virAtomicIntInc(volatile int *atomic) return InterlockedIncrement((volatile LONG *)atomic); } +static inline int +virAtomicIntDec(volatile int *atomic) +{ + return InterlockedDecrement((volatile LONG *)atomic); +} + static inline bool virAtomicIntDecAndTest(volatile int *atomic) { - return InterlockedDecrement((volatile LONG *)atomic) == 0; + return virAtomicIntDec(atomic) == 0; } static inline bool @@ -334,16 +357,22 @@ virAtomicIntInc(volatile int *atomic) return value; } -static inline bool -virAtomicIntDecAndTest(volatile int *atomic) +static inline int +virAtomicIntDec(volatile int *atomic) { - bool is_zero; + bool value; pthread_mutex_lock(&virAtomicLock); - is_zero = --(*atomic) == 0; + value = --(*atomic); pthread_mutex_unlock(&virAtomicLock); - return is_zero; + return value; +} + +static inline int +virAtomicIntDecAndTest(volatile int *atomic) +{ + return virAtomicIntDec(atomic) == 0; } static inline bool @@ -436,6 +465,8 @@ virAtomicIntXor(volatile unsigned int *atomic, virAtomicIntSet((int *)atomic, val) # define virAtomicIntInc(atomic) \ virAtomicIntInc((int *)atomic) +# define virAtomicIntDec(atomic) \ + virAtomicIntDec((int *)atomic) # define virAtomicIntDecAndTest(atomic) \ virAtomicIntDecAndTest((int *)atomic) # define virAtomicIntCompareExchange(atomic, oldval, newval) \ diff --git a/src/util/virobject.c b/src/util/virobject.c index 61b5413..4299432 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -252,9 +252,9 @@ bool virObjectUnref(void *anyobj) if (!obj) return false; - bool lastRef = virAtomicIntDecAndTest(&obj->refs); + int newRefs = virAtomicIntDec(&obj->refs); PROBE(OBJECT_UNREF, "obj=%p", obj); - if (lastRef) { + if (newRefs == 0) { PROBE(OBJECT_DISPOSE, "obj=%p", obj); virClassPtr klass = obj->klass; while (klass) { @@ -268,9 +268,13 @@ bool virObjectUnref(void *anyobj) obj->magic = 0xDEADBEEF; obj->klass = (void*)0xDEADBEEF; VIR_FREE(obj); + } else if (newRefs < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Suspicious unrefing of object %p"), + anyobj); } - return !lastRef; + return newRefs > 0; } @@ -289,8 +293,13 @@ void *virObjectRef(void *anyobj) if (!obj) return NULL; - virAtomicIntInc(&obj->refs); + bool firstRef = virAtomicIntInc(&obj->refs) <= 1; PROBE(OBJECT_REF, "obj=%p", obj); + if (firstRef) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Suspicious refing of object %p"), + anyobj); + } return anyobj; } -- 1.8.4.4

3 3

[libvirt] What to do about the qemu "-boot strict" option
by Laine Stump 28 Nov '13

28 Nov '13

Awhile back a bug was filed against libvirt about the inability to completely exclude a disk from the boot order: https://bugzilla.redhat.com/show_bug.cgi?id=888635 In short, you can't have a domain that used PXE to boot, but also has an un-bootable disk device *even if that disk isn't listed in the boot order*, because if PXE times out (e.g. due to the bridge forwarding delay), the BIOS will move on to the next target, which will be the unbootable disk device (again - even though it wasn't given a boot order), and get stuck at a "/BOOT DISK FAILURE, PRESS ANY KEY" message /until a user intervenes. It was obviously beyond the ability of libvirt to fix this (although it can be worked around by creating a very small disk image with a bootloader that merely instructs the system to reboot, and placing *that* disk in the boot order just after the PXE device), so the BZ was closed as CANTFIX. A couple days ago I noticed that Amos Kong had later actually fixed this problem in seabios and qemu: https://bugzilla.redhat.com/show_bug.cgi?id=888633 https://bugzilla.redhat.com/show_bug.cgi?id=903204 Existing behavior is preserved though, and the new behavior only comes about if "-boot strict" is specified on the qemu commandline. It definitely seems desirable to have this ability in libvirt, but I'm almost of the opinion that this should *always* be the behavior (if you want all devices to be in the boot order, you can just give all of them (or none of them, if you're feeling adventurous) a boot order ranking). But I thought it would be prudent to ask opinions about that before making any patch. So what are the opinions? Should the "if any devices are given a boot order, only attempt to boot from devices that have a boot order specified" behavior just be the default (and only) behavior when qemu/seabios supports it? (this would imply that the old behavior is just a bug)? Or do we need to make it configurable? If it needs to be configurable, the boot-related xml seems to be a bit unorganized (a flat list of elements with mostly a single attribute for each), but I suppose this could be added as a new attribute to the <bios> element...

4 4

[libvirt] [PATCH python] Improve quality of sanitytest check
by Daniel P. Berrange 28 Nov '13

28 Nov '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> Validate that every public API method is mapped into the python and that every python method has a sane C API. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- sanitytest.py | 309 +++++++++++++++++++++++++++++++++++++++++++++++++++------- setup.py | 35 +++---- 2 files changed, 294 insertions(+), 50 deletions(-) mode change 100755 => 100644 sanitytest.py diff --git a/sanitytest.py b/sanitytest.py old mode 100755 new mode 100644 index 517054b..9e4c261 --- a/sanitytest.py +++ b/sanitytest.py @@ -1,40 +1,283 @@ #!/usr/bin/python import sys +import lxml +import lxml.etree +import string +# Munge import path to insert build location for libvirt mod sys.path.insert(0, sys.argv[1]) - import libvirt +import libvirtmod + +# Path to the libvirt API XML file +xml = sys.argv[2] + +f = open(xml, "r") +tree = lxml.etree.parse(f) + +verbose = False + +wantenums = [] +wantfunctions = [] + +# Phase 1: Identify all functions and enums in public API +set = tree.xpath('/api/files/file/exports[@type="function"]/@symbol') +for n in set: + wantfunctions.append(n) + +set = tree.xpath('/api/files/file/exports[@type="enum"]/@symbol') +for n in set: + wantenums.append(n) + + +# Phase 2: Identify all classes and methods in the 'libvirt' python module +gotenums = [] +gottypes = [] +gotfunctions = { "libvirt": [] } + +for name in dir(libvirt): + if name[0] == '_': + continue + thing = getattr(libvirt, name) + if type(thing) == int: + gotenums.append(name) + elif type(thing) == type: + gottypes.append(name) + gotfunctions[name] = [] + elif callable(thing): + gotfunctions["libvirt"].append(name) + else: + pass + +for klassname in gottypes: + klassobj = getattr(libvirt, klassname) + for name in dir(klassobj): + if name[0] == '_': + continue + thing = getattr(klassobj, name) + if callable(thing): + gotfunctions[klassname].append(name) + else: + pass + + +# Phase 3: First cut at mapping of C APIs to python classes + methods +basicklassmap = {} + +for cname in wantfunctions: + name = cname + # Some virConnect APIs have stupid names + if name[0:7] == "virNode" and name[0:13] != "virNodeDevice": + name = "virConnect" + name[7:] + if name[0:7] == "virConn" and name[0:10] != "virConnect": + name = "virConnect" + name[7:] + + # The typed param APIs are only for internal use + if name[0:14] == "virTypedParams": + continue + + # These aren't functions, they're callback signatures + if name in ["virConnectAuthCallbackPtr", "virConnectCloseFunc", + "virStreamSinkFunc", "virStreamSourceFunc", "virStreamEventCallback", + "virEventHandleCallback", "virEventTimeoutCallback", "virFreeCallback"]: + continue + if name[0:21] == "virConnectDomainEvent" and name[-8:] == "Callback": + continue + + + # virEvent APIs go into main 'libvirt' namespace not any class + if name[0:8] == "virEvent": + if name[-4:] == "Func": + continue + basicklassmap[name] = ["libvirt", name, cname] + else: + found = False + # To start with map APIs to classes based on the + # naming prefix. Mistakes will be fixed in next + # loop + for klassname in gottypes: + klen = len(klassname) + if name[0:klen] == klassname: + found = True + if name not in basicklassmap: + basicklassmap[name] = [klassname, name[klen:], cname] + elif len(basicklassmap[name]) < klassname: + basicklassmap[name] = [klassname, name[klen:], cname] + + # Anything which can't map to a class goes into the + # global namespaces + if not found: + basicklassmap[name] = ["libvirt", name[3:], cname] + + +# Phase 4: Deal with oh so many special cases in C -> python mapping +finalklassmap = {} + +for name in sorted(basicklassmap): + klass = basicklassmap[name][0] + func = basicklassmap[name][1] + cname = basicklassmap[name][2] + + # The object lifecycle APIs are irrelevant since they're + # used inside the object constructors/destructors. + if func in ["Ref", "Free", "New", "GetConnect", "GetDomain"]: + if klass == "virStream" and func == "New": + klass = "virConnect" + func = "NewStream" + else: + continue + + + # All the error handling methods need special handling + if klass == "libvirt": + if func in ["CopyLastError", "DefaultErrorFunc", + "ErrorFunc", "FreeError", + "SaveLastError", "ResetError"]: + continue + elif func in ["GetLastError", "GetLastErrorMessage", "ResetLastError", "Initialize"]: + func = "vir" + func + elif func == "SetErrorFunc": + func = "RegisterErrorHandler" + elif klass == "virConnect": + if func in ["CopyLastError", "SetErrorFunc"]: + continue + elif func in ["GetLastError", "ResetLastError"]: + func = "virConn" + func + + # Remove 'Get' prefix from most APIs, except those in virConnect + # and virDomainSnapshot namespaces which stupidly used a different + # convention which we now can't fix without breaking API + if func[0:3] == "Get" and klass not in ["virConnect", "virDomainSnapshot", "libvirt"]: + if func not in ["GetCPUStats"]: + func = func[3:] + + # The object creation and lookup APIs all have to get re-mapped + # into the parent class + if func in ["CreateXML", "CreateLinux", "CreateXMLWithFiles", + "DefineXML", "CreateXMLFrom", "LookupByUUID", + "LookupByUUIDString", "LookupByVolume" "LookupByName", + "LookupByID", "LookupByName", "LookupByKey", "LookupByPath", + "LookupByMACString", "LookupByUsage", "LookupByVolume", + "LookupSCSIHostByWWN", "Restore", "RestoreFlags", + "SaveImageDefineXML", "SaveImageGetXMLDesc"]: + if klass != "virDomain": + func = klass[3:] + func + + if klass == "virDomainSnapshot": + klass = "virDomain" + func = func[6:] + elif klass == "virStorageVol" and func in ["StorageVolCreateXMLFrom", "StorageVolCreateXML"]: + klass = "virStoragePool" + func = func[10:] + elif func == "StoragePoolLookupByVolume": + klass = "virStorageVol" + elif func == "StorageVolLookupByName": + klass = "virStoragePool" + else: + klass = "virConnect" + + # The open methods get remapped to primary namespace + if klass == "virConnect" and func in ["Open", "OpenAuth", "OpenReadOnly"]: + klass = "libvirt" + + # These are inexplicably renamed in the python API + if func == "ListDomains": + func = "ListDomainsID" + elif func == "ListAllNodeDevices": + func = "ListAllDevices" + elif func == "ListNodeDevices": + func = "ListDevices" + + # The virInterfaceChangeXXXX APIs go into virConnect. Stupidly + # they have lost their 'interface' prefix in names, but we can't + # fix this name + if func[0:6] == "Change": + klass = "virConnect" + + # Need to special case the snapshot APIs + if klass == "virDomainSnapshot" and func in ["Current", "ListNames", "Num"]: + klass = "virDomain" + func = "snapshot" + func + + # Names should stsart with lowercase letter... + func = string.lower(func[0:1]) + func[1:] + if func[0:8] == "nWFilter": + func = "nwfilter" + func[8:] + + # ...except when they don't. More stupid naming + # decisions we can't fix + if func == "iD": + func = "ID" + if func == "uUID": + func = "UUID" + if func == "uUIDString": + func = "UUIDString" + if func == "oSType": + func = "OSType" + if func == "xMLDesc": + func = "XMLDesc" + if func == "mACString": + func = "MACString" + + finalklassmap[name] = [klass, func, cname] + + +# Phase 5: Validate sure that every C API is mapped to a python API +fail = False +usedfunctions = {} +for name in sorted(finalklassmap): + klass = finalklassmap[name][0] + func = finalklassmap[name][1] + + if func in gotfunctions[klass]: + usedfunctions["%s.%s" % (klass, func)] = 1 + if verbose: + print "PASS %s -> %s.%s" % (name, klass, func) + else: + print "FAIL %s -> %s.%s (C API not mapped to python)" % (name, klass, func) + fail = True + + +# Phase 6: Validate that every python API has a corresponding C API +for klass in gotfunctions: + if klass == "libvirtError": + continue + for func in sorted(gotfunctions[klass]): + # These are pure python methods with no C APi + if func in ["connect", "getConnect", "domain", "getDomain"]: + continue + + key = "%s.%s" % (klass, func) + if not key in usedfunctions: + print "FAIL %s.%s (Python API not mapped to C)" % (klass, func) + fail = True + else: + if verbose: + print "PASS %s.%s" % (klass, func) + +# Phase 7: Validate that all the low level C APIs have binding +for name in sorted(finalklassmap): + cname = finalklassmap[name][2] + + pyname = cname + if pyname == "virSetErrorFunc": + pyname = "virRegisterErrorHandler" + elif pyname == "virConnectListDomains": + pyname = "virConnectListDomainsID" + + # These exist in C and exist in python, but we've got + # a pure-python impl so don't check them + if name in ["virStreamRecvAll", "virStreamSendAll"]: + continue + + try: + thing = getattr(libvirtmod, pyname) + except AttributeError: + print "FAIL libvirtmod.%s (C binding does not exist)" % pyname + fail = True -globals = dir(libvirt) - -# Sanity test that the generator hasn't gone wrong - -# Look for core classes -for clsname in ["virConnect", - "virDomain", - "virDomainSnapshot", - "virInterface", - "virNWFilter", - "virNodeDevice", - "virNetwork", - "virSecret", - "virStoragePool", - "virStorageVol", - "virStream", - ]: - assert(clsname in globals) - assert(object in getattr(libvirt, clsname).__bases__) - -# Constants -assert("VIR_CONNECT_RO" in globals) - -# Error related bits -assert("libvirtError" in globals) -assert("VIR_ERR_AUTH_FAILED" in globals) -assert("virGetLastError" in globals) - -# Some misc methods -assert("virInitialize" in globals) -assert("virEventAddHandle" in globals) -assert("virEventRegisterDefaultImpl" in globals) +if fail: + sys.exit(1) +else: + sys.exit(0) diff --git a/setup.py b/setup.py index 17b4722..bf222f8 100755 --- a/setup.py +++ b/setup.py @@ -59,6 +59,20 @@ def get_pkgconfig_data(args, mod, required=True): return line +def get_api_xml_files(): + """Check with pkg-config that libvirt is present and extract + the API XML file paths we need from it""" + + libvirt_api = get_pkgconfig_data(["--variable", "libvirt_api"], "libvirt") + + offset = libvirt_api.index("-api.xml") + libvirt_qemu_api = libvirt_api[0:offset] + "-qemu-api.xml" + + offset = libvirt_api.index("-api.xml") + libvirt_lxc_api = libvirt_api[0:offset] + "-lxc-api.xml" + + return (libvirt_api, libvirt_qemu_api, libvirt_lxc_api) + ldflags = get_pkgconfig_data(["--libs-only-L"], "libvirt", False) cflags = get_pkgconfig_data(["--cflags"], "libvirt", False) @@ -105,23 +119,8 @@ if have_libvirt_lxc: class my_build(build): - def get_api_xml_files(self): - """Check with pkg-config that libvirt is present and extract - the API XML file paths we need from it""" - - libvirt_api = get_pkgconfig_data(["--variable", "libvirt_api"], "libvirt") - - offset = libvirt_api.index("-api.xml") - libvirt_qemu_api = libvirt_api[0:offset] + "-qemu-api.xml" - - offset = libvirt_api.index("-api.xml") - libvirt_lxc_api = libvirt_api[0:offset] + "-lxc-api.xml" - - return (libvirt_api, libvirt_qemu_api, libvirt_lxc_api) - - def run(self): - apis = self.get_api_xml_files() + apis = get_api_xml_files() self.spawn(["python", "generator.py", "libvirt", apis[0]]) self.spawn(["python", "generator.py", "libvirt-qemu", apis[1]]) @@ -266,7 +265,9 @@ class my_test(Command): Run test suite """ - self.spawn(["python", "sanitytest.py", self.build_platlib]) + apis = get_api_xml_files() + + self.spawn(["python", "sanitytest.py", self.build_platlib, apis[0]]) class my_clean(clean): -- 1.8.3.1

3 4

[libvirt] [PATCH 0/3] Fix unmounting of dirs in LXC code
by Daniel P. Berrange 28 Nov '13

28 Nov '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> Dan Walsh identified that if you ask LXC to mount both /etc/aliases and /etc/aliases.db in the container you only end up with one of them mounted. This was due to a bogus subpath check. It is a one line fix which turned into a much bigger series so that I could add a test case for this code. Daniel P. Berrange (3): Remove 'abs_srcdir' variable from test files Pull lxcContainerGetSubtree out into shared virfile module Fix bug in identifying sub-mounts .gitignore | 1 + src/libvirt_private.syms | 4 ++ src/lxc/lxc_container.c | 63 ++------------------- src/util/virfile.c | 114 +++++++++++++++++++++++++++++++++++++ src/util/virfile.h | 9 +++ src/util/virstring.c | 29 ++++++++++ src/util/virstring.h | 3 + tests/Makefile.am | 7 +++ tests/testutils.c | 12 ---- tests/testutils.h | 1 - tests/virfiledata/mounts1.txt | 31 ++++++++++ tests/virfiledata/mounts2.txt | 33 +++++++++++ tests/virfiletest.c | 128 ++++++++++++++++++++++++++++++++++++++++++ tests/virpcimock.c | 5 -- 14 files changed, 363 insertions(+), 77 deletions(-) create mode 100644 tests/virfiledata/mounts1.txt create mode 100644 tests/virfiledata/mounts2.txt create mode 100644 tests/virfiletest.c -- 1.8.3.1

2 7

[libvirt] [PATCH] qemu: add support for -device pvpanic
by Hu Tao 28 Nov '13

28 Nov '13

qemu removes the builtin pvpanic device for all qemu versions since 1.7, in order to support <on_crash>, '-device pvpanic' has to be added to qemu command line. Signed-off-by: Hu Tao <hutao(a)cn.fujitsu.com> --- src/qemu/qemu_capabilities.c | 8 ++++++++ src/qemu/qemu_capabilities.h | 2 ++ src/qemu/qemu_command.c | 4 ++++ 3 files changed, 14 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 548b988..7783997 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -243,6 +243,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "virtio-mmio", "ich9-intel-hda", "kvm-pit-lost-tick-policy", + + "pvpanic", /* 160 */ ); struct _virQEMUCaps { @@ -1198,6 +1200,9 @@ virQEMUCapsComputeCmdFlags(const char *help, virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_SHARE_POLICY); } + if (version >= 1005000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_PVPANIC); + return 0; } @@ -2561,6 +2566,9 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (qemuCaps->version >= 1006000) virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); + if (qemuCaps->version >= 1005000) + virQEMUCapsSet(qemuCaps, QEMU_CAPS_PVPANIC); + if (virQEMUCapsProbeQMPCommands(qemuCaps, mon) < 0) goto cleanup; if (virQEMUCapsProbeQMPEvents(qemuCaps, mon) < 0) diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 02d47c6..06d2fac 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -199,6 +199,8 @@ enum virQEMUCapsFlags { QEMU_CAPS_DEVICE_ICH9_INTEL_HDA = 158, /* -device ich9-intel-hda */ QEMU_CAPS_KVM_PIT_TICK_POLICY = 159, /* kvm-pit.lost_tick_policy */ + QEMU_CAPS_PVPANIC = 160, /* -device pvpanic */ + QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 763417f..b1307a3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9588,6 +9588,10 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PVPANIC)) { + virCommandAddArgList(cmd, "-device", "pvpanic", NULL); + } + if (mlock) { unsigned long long memKB; -- 1.8.3.1

4 5

[libvirt] [PATCH] libvirt-perl: Fix the wrong binding of virNodeDeviceLookupSCSIHostByWWN
by Osier Yang 28 Nov '13

28 Nov '13

The second parameter for virNodeDeviceLookupSCSIHostByWWN is "wwnn" instead of "wwpn", and the API supports "flags". --- AUTHORS | 1 + lib/Sys/Virt/NodeDevice.pm | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/AUTHORS b/AUTHORS index d53f191..c2af49a 100644 --- a/AUTHORS +++ b/AUTHORS @@ -12,5 +12,6 @@ Patches contributed by: Stepan Kasal <skasal-at-redhat-dot-com> Ludwig Nussel <ludwig-dot-nussel-at-suse-dot-de> Zhe Peng <zpeng-at-redhat-dot-com> + Osier Yang <jyang-at-redhat-dot-com> -- End diff --git a/lib/Sys/Virt/NodeDevice.pm b/lib/Sys/Virt/NodeDevice.pm index 984910d..29ceac7 100644 --- a/lib/Sys/Virt/NodeDevice.pm +++ b/lib/Sys/Virt/NodeDevice.pm @@ -51,10 +51,11 @@ sub _new { my $self; if (exists $params{name}) { $self = Sys::Virt::NodeDevice::_lookup_by_name($con, $params{name}); - } elsif (exists $params{wwpn}) { + } elsif (exists $params{wwnn}) { $self = Sys::Virt::NodeDevice::_lookup_scsihost_by_wwn($con, + $params{wwnn}, $params{wwpn}, - $params{wwnn}); + $params{flags}); } elsif (exists $params{xml}) { $self = Sys::Virt::NodeDevice::_create_xml($con, $params{xml}); } else { -- 1.8.1.4

2 2

[libvirt] [test-API][PATCH V2] Add blockjob related test functions and cases
by Jincheng Miao 28 Nov '13

28 Nov '13

* repos/domain/blkstatsflags.py * repos/domain/block_iotune.py * repos/domain/block_peek.py * repos/domain/block_resize.py * repos/domain/domain_blkio.py * cases/basic_blockjob.conf V1->V2: Removed diskpath params for block_resize, this hard code disk device is not convenient for users. --- cases/basic_blockjob.conf | 83 +++++++++++++++++++++ repos/domain/blkstatsflags.py | 63 ++++++++++++++++ repos/domain/block_iotune.py | 118 ++++++++++++++++++++++++++++++ repos/domain/block_peek.py | 69 ++++++++++++++++++ repos/domain/block_resize.py | 94 ++++++++++++++++++++++++ repos/domain/domain_blkio.py | 163 ++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 590 insertions(+) create mode 100644 cases/basic_blockjob.conf create mode 100644 repos/domain/blkstatsflags.py create mode 100644 repos/domain/block_iotune.py create mode 100644 repos/domain/block_peek.py create mode 100644 repos/domain/block_resize.py create mode 100644 repos/domain/domain_blkio.py diff --git a/cases/basic_blockjob.conf b/cases/basic_blockjob.conf new file mode 100644 index 0000000..21332dc --- /dev/null +++ b/cases/basic_blockjob.conf @@ -0,0 +1,83 @@ +domain:install_linux_cdrom + guestname + $defaultname + guestos + $defaultos + guestarch + $defaultarch + vcpu + $defaultvcpu + memory + $defaultmem + hddriver + $defaulthd + nicdriver + $defaultnic + macaddr + 54:52:00:45:c3:8a + +domain:install_linux_check + guestname + $defaultname + virt_type + $defaulthv + hddriver + $defaulthd + nicdriver + $defaultnic + +domain:block_iotune + guestname + $defaultname + bytes_sec + 100000 + iops_sec + 0 + +domain:block_iotune + guestname + $defaultname + bytes_sec + 0 + iops_sec + 1000 + +domain:block_peek + guestname + $defaultname + +domain:block_peek + guestname + $defaultname + +domain:block_resize + guestname + $defaultname + disksize + 1G + +domain:blkstats + guestname + $defaultname + +domain:blkstatsflags + guestname + $defaultname + flags + 0 + +domain:domain_blkinfo + guestname + $defaultname + +domain:domain_blkio + guestname + $defaultname + weight + 500 + +domain:undefine + guestname + $defaultname + +options cleanup=enable diff --git a/repos/domain/blkstatsflags.py b/repos/domain/blkstatsflags.py new file mode 100644 index 0000000..4c84a18 --- /dev/null +++ b/repos/domain/blkstatsflags.py @@ -0,0 +1,63 @@ +#!/usr/bin/evn python +# To test domain block device statistics with flags + +import time +import libxml2 + +import libvirt +from libvirt import libvirtError + +from src import sharedmod + +required_params = ('guestname', 'flags') +optional_params = {} + +def check_guest_status(domobj): + """Check guest current status""" + state = domobj.info()[0] + if state == libvirt.VIR_DOMAIN_SHUTOFF or state == libvirt.VIR_DOMAIN_SHUTDOWN: + # add check function + return False + else: + return True + +def check_blkstats(): + """Check block device statistic result""" + pass + +def blkstatsflags(params): + """Domain block device statistic""" + logger = params['logger'] + guestname = params['guestname'] + flags = int(params['flags']) + + conn = sharedmod.libvirtobj['conn'] + + domobj = conn.lookupByName(guestname) + + # Check domain block status + if check_guest_status(domobj): + pass + else: + domobj.create() + time.sleep(90) + try: + xml = domobj.XMLDesc(0) + doc = libxml2.parseDoc(xml) + cont = doc.xpathNewContext() + devs = cont.xpathEval("/domain/devices/disk/target/@dev") + + for dev in devs: + path = dev.content + blkstats = domobj.blockStatsFlags(path, flags) + # check_blkstats() + logger.debug(blkstats) + for entry in blkstats.keys(): + logger.info("%s %s %s" %(path, entry, blkstats[entry])) + + except libvirtError, e: + logger.error("API error message: %s, error code is %s" + % (e.message, e.get_error_code())) + return 1 + + return 0 diff --git a/repos/domain/block_iotune.py b/repos/domain/block_iotune.py new file mode 100644 index 0000000..26e4823 --- /dev/null +++ b/repos/domain/block_iotune.py @@ -0,0 +1,118 @@ +#!/usr/bin/evn python +# To test domain block device iotune + +import time +import libxml2 +import libvirt +from libvirt import libvirtError + +from src import sharedmod + +required_params = ('guestname', 'bytes_sec', 'iops_sec') +optional_params = {} + +def check_guest_status(domobj): + """Check guest current status""" + state = domobj.info()[0] + if state == libvirt.VIR_DOMAIN_SHUTOFF or \ + state == libvirt.VIR_DOMAIN_SHUTDOWN: + # add check function + return False + else: + return True + +def prepare_block_iotune(param, wbs, rbs, tbs, wis, ris, tis, logger): + """prepare the block iotune parameter + """ + logger.info("write_bytes_sec : %s" % wbs) + param['write_bytes_sec'] = wbs + logger.info("read_bytes_sec : %s" % rbs) + param['read_bytes_sec'] = rbs + logger.info("total_bytes_sec : %s" % tbs) + param['total_bytes_sec'] = tbs + logger.info("write_iops_sec : %s" % wis) + param['write_iops_sec'] = wis + logger.info("read_iops_sec : %s" % ris) + param['read_iops_sec'] = ris + logger.info("total_iops_sec : %s\n" % tis) + param['total_iops_sec'] = tis + return 0 + +def check_iotune(expected_param, result_param): + """check block iotune configuration + """ + for k in expected_param.keys(): + if expected_param[k] != result_param[k]: + return 1 + return 0 + +def block_iotune(params): + """Domain block device iotune""" + logger = params['logger'] + guestname = params['guestname'] + bytes_sec = int(params['bytes_sec']) + iops_sec = int(params['iops_sec']) + flag = 0 + + conn = sharedmod.libvirtobj['conn'] + + domobj = conn.lookupByName(guestname) + + # Check domain block status + if check_guest_status(domobj): + pass + else: + domobj.create() + time.sleep(90) + + try: + xml = domobj.XMLDesc(0) + doc = libxml2.parseDoc(xml) + cont = doc.xpathNewContext() + vdevs = cont.xpathEval("/domain/devices/disk/target/@dev") + vdev = vdevs[0].content + + iotune_para = {'write_bytes_sec': 0L, + 'total_iops_sec': 0L, + 'read_iops_sec': 0L, + 'read_bytes_sec': 0L, + 'write_iops_sec': 0L, + 'total_bytes_sec': 0L + } + + logger.info("prepare block iotune:") + prepare_block_iotune(iotune_para, bytes_sec, bytes_sec, 0, + iops_sec, iops_sec, 0, logger) + + logger.info("start to set block iotune:") + domobj.setBlockIoTune(vdev, iotune_para, flag) + + res = domobj.blockIoTune(vdev, flag) + ret = check_iotune(iotune_para, res) + if not ret: + logger.info("set pass") + else: + logger.error("fails to set") + return 1 + + logger.info("prepare block iotune:") + prepare_block_iotune(iotune_para, 0, 0, bytes_sec, + 0, 0, iops_sec, logger) + + logger.info("start to set block iotune:") + domobj.setBlockIoTune(vdev, iotune_para, flag) + + res = domobj.blockIoTune(vdev, flag) + ret = check_iotune(iotune_para, res) + if not ret: + logger.info("set pass") + else: + logger.error("fails to set") + return 1 + + except libvirtError, e: + logger.error("API error message: %s, error code is %s" + % (e.message, e.get_error_code())) + return 1 + + return 0 diff --git a/repos/domain/block_peek.py b/repos/domain/block_peek.py new file mode 100644 index 0000000..b88668a --- /dev/null +++ b/repos/domain/block_peek.py @@ -0,0 +1,69 @@ +#!/usr/bin/evn python +# To test domain block device peek + +import time +import libxml2 +import libvirt +from libvirt import libvirtError + +from src import sharedmod + +required_params = ('guestname',) +optional_params = {} + +def check_guest_status(domobj): + """Check guest current status""" + state = domobj.info()[0] + if state == libvirt.VIR_DOMAIN_SHUTOFF or \ + state == libvirt.VIR_DOMAIN_SHUTDOWN: + # add check function + return False + else: + return True + +def block_peek(params): + """domain block peek test function + """ + logger = params['logger'] + guestname = params['guestname'] + flag = 0 + + conn = sharedmod.libvirtobj['conn'] + + domobj = conn.lookupByName(guestname) + + # Check domain block status + if check_guest_status(domobj): + pass + else: + domobj.create() + time.sleep(90) + + try: + xml = domobj.XMLDesc(0) + doc = libxml2.parseDoc(xml) + cont = doc.xpathNewContext() + vdevs = cont.xpathEval("/domain/devices/disk/target/@dev") + vdev = vdevs[0].content + + logger.info("start to test block_peek.") + logger.info("get the MBR's last byte of domain %s %s is:" + % (guestname, vdev)) + + last_byte = domobj.blockPeek(vdev, 511, 1, flag) + logger.info(last_byte) + + # compare with '\xaa' + if last_byte == '\xaa': + logger.info("Pass: the last byte is \\xaa") + else: + logger.error("Failed: the last byte is not \\xaa") + logger.error("please make sure the guest is bootable") + return 1 + + except libvirtError, e: + logger.error("API error message: %s, error code is %s" + % (e.message, e.get_error_code())) + return 1 + + return 0 diff --git a/repos/domain/block_resize.py b/repos/domain/block_resize.py new file mode 100644 index 0000000..6070c22 --- /dev/null +++ b/repos/domain/block_resize.py @@ -0,0 +1,94 @@ +#!/usr/bin/evn python +# To test domain block device resize + +import time +import libxml2 +import libvirt +from libvirt import libvirtError + +from src import sharedmod +from utils import utils + +required_params = ('guestname', 'disksize',) +optional_params = {} + +def check_guest_status(domobj): + """Check guest current status""" + state = domobj.info()[0] + if state == libvirt.VIR_DOMAIN_SHUTOFF or \ + state == libvirt.VIR_DOMAIN_SHUTDOWN: + # add check function + return False + else: + return True + +def block_resize(params): + """domain block resize test function + """ + logger = params['logger'] + guestname = params['guestname'] + disksize = params['disksize'] + flag = 0 + + out = utils.get_capacity_suffix_size(disksize) + if len(out) == 0: + logger.error("disksize parse error: \'%s\'" % disksize) + logger.error("disksize should be a number with capacity suffix") + return 1 + + if out['suffix'] == 'K': + flag = 0 + disksize = long(out['capacity']) + elif out['suffix'] == 'B': + flag = 1 + disksize = long(out['capacity_byte']) + elif out['suffix'] == 'M': + flag = 0 + disksize = long(out['capacity']) * 1024 + elif out['suffix'] == 'G': + flag = 0 + disksize = long(out['capacity']) * 1024 * 1024 + else: + logger.error("disksize parse error: with a unsupported suffix \'%s\'" + % out['suffix']) + logger.error("the available disksize suffix of block_resize is: ") + logger.error("B, K, M, G, T") + return 1 + + conn = sharedmod.libvirtobj['conn'] + + domobj = conn.lookupByName(guestname) + + xml = domobj.XMLDesc(0) + doc = libxml2.parseDoc(xml) + cont = doc.xpathNewContext() + vdevs = cont.xpathEval("/domain/devices/disk/source/@file") + diskpath = vdevs[0].content + logger.info("the disk is %s" % diskpath) + + # Check domain block status + if check_guest_status(domobj): + pass + else: + domobj.create() + time.sleep(90) + + try: + logger.info("resize domain disk to %s" % disksize) + domobj.blockResize(diskpath, disksize, flag) + + # Currently, the units of disksize which get from blockInfo is byte. + block_info = domobj.blockInfo(diskpath, 0) + + if block_info[0] == disksize * (1 + 1023 * (1 - flag)): + logger.info("domain disk resize success") + else: + logger.error("error: domain disk change into %s" % block_info[0]) + return 1 + + except libvirtError, e: + logger.error("API error message: %s, error code is %s" + % (e.message, e.get_error_code())) + return 1 + + return 0 diff --git a/repos/domain/domain_blkio.py b/repos/domain/domain_blkio.py new file mode 100644 index 0000000..f1c637d --- /dev/null +++ b/repos/domain/domain_blkio.py @@ -0,0 +1,163 @@ +#!/usr/bin/evn python +# To test domain blkio parameters + +import os +import time +import libxml2 +import libvirt +import commands +from libvirt import libvirtError + +from src import sharedmod + +CGROUP_PATH = "/cgroup" +BLKIO_PATH1 = "%s/blkio/libvirt/qemu/%s" +BLKIO_PATH2 = "/sys/fs%s/blkio/machine/%s.libvirt-qemu" +GET_PARTITION = "df -P %s | tail -1 | awk {'print $1'}" + +required_params = ('guestname', 'weight',) +optional_params = {} + +def get_output(command, logger): + """execute shell command + """ + status, ret = commands.getstatusoutput(command) + if status: + logger.error("executing "+ "\"" + command + "\"" + " failed") + logger.error(ret) + return status, ret + +def get_device(domobj, logger): + """get the disk device which domain image stored in + """ + xml = domobj.XMLDesc(0) + doc = libxml2.parseDoc(xml) + cont = doc.xpathNewContext() + devs = cont.xpathEval("/domain/devices/disk/source/@file") + image_file = devs[0].content + + status, output = get_output(GET_PARTITION % image_file, logger) + if not status: + return output[:-1] + else: + logger.error("get device error: ") + logger.error(GET_PARTITION % image_file) + return "" + +def check_blkio_paras(domain_blkio_path, domainname, blkio_paras, logger): + """check blkio parameters according to cgroup filesystem + """ + logger.info("checking blkio parameters from cgroup") + if 'weight' in blkio_paras: + expected_weight = blkio_paras['weight'] + status, output = get_output("cat %s/blkio.weight" + % domain_blkio_path, logger) + if not status: + logger.info("%s/blkio.weight is \"%s\"" + % (domain_blkio_path, output)) + else: + return 1 + + if int(output) == expected_weight: + logger.info("the weight matches with cgroup blkio.weight") + return 0 + else: + logger.error("the weight mismatches with cgroup blkio.weight") + return 1 + + if 'device_weight' in blkio_paras: + expected_device_weight = blkio_paras['device_weight'] + status, output = get_output("cat %s/blkio.weight_device" + % domain_blkio_path, logger) + if not status: + logger.info("%s/blkio.weight_device is \"%s\"" + % (domain_blkio_path, output)) + else: + return 1 + + if output.split(' ')[1] == expected_device_weight.split(',')[1]: + logger.info("the device_weight matches with cgroup \ + blkio.weight_device") + return 0 + else: + logger.error("the device_weight mismatches with cgroup \ + blkio.weight_device") + return 1 + + return 0 + +def check_guest_status(domobj): + """Check guest current status""" + state = domobj.info()[0] + if state == libvirt.VIR_DOMAIN_SHUTOFF or \ + state == libvirt.VIR_DOMAIN_SHUTDOWN: + # add check function + return False + else: + return True + +def domain_blkio(params): + """domain blkio parameters test function""" + logger = params['logger'] + guestname = params['guestname'] + expected_weight = params['weight'] + flag = 0 + + conn = sharedmod.libvirtobj['conn'] + + domobj = conn.lookupByName(guestname) + + # Check domain block status + if check_guest_status(domobj): + pass + else: + domobj.create() + time.sleep(90) + + if os.path.exists(CGROUP_PATH): + blkio_path = BLKIO_PATH1 % (CGROUP_PATH, guestname) + else: + blkio_path = BLKIO_PATH2 % (CGROUP_PATH, guestname) + + try: + blkio_paras = domobj.blkioParameters(flag) + + logger.info("the blkio weight of %s is: %d" + % (guestname, blkio_paras['weight'])) + + status = check_blkio_paras(blkio_path, guestname, blkio_paras, + logger) + if status != 0: + return 1 + + logger.info("start to set param weight to %s" % expected_weight) + blkio_paras = {'weight':int(expected_weight)} + status = domobj.setBlkioParameters(blkio_paras, flag) + if status != 0: + return 1 + + status = check_blkio_paras(blkio_path, guestname, blkio_paras, + logger) + if status != 0: + return 1 + + device = get_device(domobj, logger) + device_weight = "%s,%s" % (device, expected_weight) + logger.info("start to set param device_weight to %s" + % device_weight) + blkio_paras = {'device_weight':device_weight} + status = domobj.setBlkioParameters(blkio_paras, flag) + if status != 0: + return 1 + + status = check_blkio_paras(blkio_path, guestname, blkio_paras, + logger) + if status != 0: + return 1 + + except libvirtError, e: + logger.error("API error message: %s, error code is %s" + % (e.message, e.get_error_code())) + return 1 + + return 0 -- 1.8.3.1

2 3

[libvirt] Entering freeze for 1.2.0, rc1 available
by Daniel Veillard 28 Nov '13

28 Nov '13

So as planned we are entering freeze for libvirt-1.2.0, I just tagged the git trees - plural as we now use libvirt-python separate git for this binding - and pushed tarballs and rpms to the usual place at: ftp://libvirt.org/libvirt/ it did allows a correct update at the rpm level for me and virt-manager does work correctly at least with my minimal testing. We now need further reviews, obviously on the portability front especially as the python bindings are a separate tarball. It would be cool to get more testing on specific aspects of the release too, like the gluster pool and VBox 4.3 support. Give it a try, thanks ! Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veillard(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/

2 3

[libvirt] [PATCH] tests: Make pci config files writable
by Michal Privoznik 27 Nov '13

27 Nov '13

As of 21685c955 the 'distcheck' is broken. The problem is, by default it copies all the necessary files and make them read only. However, pci device detach test doesn't work that way. The PCI device configs are stored within our tests/ directory and need to be writable (detaching and resetting means writing into the config file). Hence, we must make those files writable again. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- tests/Makefile.am | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/Makefile.am b/tests/Makefile.am index e46d5f7..29dbf76 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -921,3 +921,8 @@ endif ! WITH_CIL CLEANFILES = *.cov *.gcov .libs/*.gcda .libs/*.gcno *.gcno *.gcda *.cmi *.cmx \ object-locking-files.txt + +# Some tests tend to write into files. Notably, the virpcitest, which detach +# and reset a pci device (achieved byt writing into a pci config file). +check-local: + chmod -R u+w $(srcdir)/virpcitestdata/ -- 1.8.4.4

3 3

[libvirt] [PATCH] network: properly update iptables rules during net-update
by Laine Stump 27 Nov '13

27 Nov '13

This patch resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1035336 The basic problem is that during a network update, the required iptables rules sometimes change, and this was being handled by simply removing and re-adding the rules. However, the removal of the old rules was done based on the *new* state of the network, which would mean that some of the rules would not match those currently in the system, so the old rules wouldn't be removed. This patch removes the old rules prior to updating the network definitionm then adds the new rules as soon as the definition is updated. Note that this could lead to a stray packet or two during the interim, but that was already a problem before (the period of limbo is now just slightly longer). While moving the location for the rules, I added a few more sections that should result in the iptables rules being redone: DHCP_RANGE and DHCP_HOST - these are needed because adding/removing a dhcp host entry could lead to the dhcp service being started/stopped, which would require that the mangle rule that fixes up dhcp response checksums sould need to be added/removed, and this wasn't being done. --- src/network/bridge_driver.c | 47 +++++++++++++++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 83dc931..d7f12e1 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2642,6 +2642,7 @@ networkUpdate(virNetworkPtr net, size_t i; virNetworkIpDefPtr ipdef; bool oldDhcpActive = false; + bool needFirewallRefresh = false; virCheckFlags(VIR_NETWORK_UPDATE_AFFECT_LIVE | @@ -2683,8 +2684,39 @@ networkUpdate(virNetworkPtr net, flags |= VIR_NETWORK_UPDATE_AFFECT_CONFIG; } + if (isActive && (flags & VIR_NETWORK_UPDATE_AFFECT_LIVE)) { + /* Take care of anything that must be done before updating the + * live NetworkDef. + */ + if (network->def->forward.type == VIR_NETWORK_FORWARD_NONE || + network->def->forward.type == VIR_NETWORK_FORWARD_NAT || + network->def->forward.type == VIR_NETWORK_FORWARD_ROUTE) { + switch (section) { + case VIR_NETWORK_SECTION_FORWARD: + case VIR_NETWORK_SECTION_FORWARD_INTERFACE: + case VIR_NETWORK_SECTION_IP: + case VIR_NETWORK_SECTION_IP_DHCP_RANGE: + case VIR_NETWORK_SECTION_IP_DHCP_HOST: + /* these could affect the firewall rules, so remove the + * old rules (and remember to load new ones after the + * update). + */ + networkRemoveFirewallRules(network); + needFirewallRefresh = true; + default: + break; + } + } + } + /* update the network config in memory/on disk */ - if (virNetworkObjUpdate(network, command, section, parentIndex, xml, flags) < 0) + if (virNetworkObjUpdate(network, command, section, parentIndex, xml, flags) < 0) { + if (needFirewallRefresh) + ignore_value(networkAddFirewallRules(network)); + goto cleanup; + } + + if (needFirewallRefresh && networkAddFirewallRules(network) < 0) goto cleanup; if (flags & VIR_NETWORK_UPDATE_AFFECT_CONFIG) { @@ -2754,19 +2786,6 @@ networkUpdate(virNetworkPtr net, goto cleanup; } - if ((section == VIR_NETWORK_SECTION_IP || - section == VIR_NETWORK_SECTION_FORWARD || - section == VIR_NETWORK_SECTION_FORWARD_INTERFACE) && - (network->def->forward.type == VIR_NETWORK_FORWARD_NONE || - network->def->forward.type == VIR_NETWORK_FORWARD_NAT || - network->def->forward.type == VIR_NETWORK_FORWARD_ROUTE)) { - /* these could affect the iptables rules */ - networkRemoveFirewallRules(network); - if (networkAddFirewallRules(network) < 0) - goto cleanup; - - } - /* save current network state to disk */ if ((ret = virNetworkSaveStatus(driverState->stateDir, network)) < 0) { -- 1.8.4.2

2 1

[libvirt] [PATCH] tests: fix virpcitest with read-only srcdir
by Eric Blake 27 Nov '13

27 Nov '13

'make distcheck' has been broken since commit 21685c9; basically, it emulates the case of a read-only $(srcdir) (such as building from a tarball exploded onto a CD-ROM), but we were creating our fake pci device as a symlink into $(srcdir) and failing when that requires opening the config file for writing: 3) testVirPCIDeviceReset ... libvirt: error : Failed to open config space file '/sys/bus/pci/devices/0000:00:01.0/config': Permission denied Fix it by copying rather than symlinking. * tests/virpcimock.c (make_file): Add parameter to allow binary creation; adjust all callers. (pci_device_new_from_stub): Copy rather than symlink. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- Pushing under the build-breaker rule, as an alternative to the earlier attempt that I nak'ed: https://www.redhat.com/archives/libvir-list/2013-November/msg00999.html tests/virpcimock.c | 43 +++++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/tests/virpcimock.c b/tests/virpcimock.c index 48d5f16..a5cef46 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -154,10 +154,13 @@ static int pci_driver_handle_remove_id(const char *path); static void make_file(const char *path, const char *name, - const char *value) + const char *value, + ssize_t len) { int fd = -1; char *filepath = NULL; + if (value && len == -1) + len = strlen(value); if (virAsprintfQuiet(&filepath, "%s/%s", path, name) < 0) ABORT_OOM(); @@ -165,7 +168,7 @@ make_file(const char *path, if ((fd = realopen(filepath, O_CREAT|O_WRONLY, 0666)) < 0) ABORT("Unable to open: %s", filepath); - if (value && safewrite(fd, value, strlen(value)) != strlen(value)) + if (value && safewrite(fd, value, len) != len) ABORT("Unable to write: %s", filepath); VIR_FORCE_CLOSE(fd); @@ -302,7 +305,7 @@ pci_device_new_from_stub(const struct pciDevice *data) { struct pciDevice *dev; char *devpath; - char *configSrc, *configDst; + char *configSrc; char tmp[32]; struct stat sb; @@ -321,25 +324,32 @@ pci_device_new_from_stub(const struct pciDevice *data) * symlink it. Otherwise create a dummy config file. */ if ((realstat && realstat(configSrc, &sb) == 0) || (real__xstat && real__xstat(_STAT_VER, configSrc, &sb) == 0)) { - /* On success make symlink to @configSrc */ - if (virAsprintfQuiet(&configDst, "%s/config", devpath) < 0) - ABORT_OOM(); - - if (symlink(configSrc, configDst) < 0) - ABORT("Unable to create symlink: %s", configDst); + /* On success, copy @configSrc into the destination (a copy, + * rather than a symlink, is required since we write into the + * file, and parallel VPATH builds must not stomp on the + * original; besides, 'make distcheck' requires the original + * to be read-only */ + char *buf; + ssize_t len; + + if ((len = virFileReadAll(configSrc, 4096, &buf)) < 0) + ABORT("Unable to read config file '%s'", configSrc); + + make_file(devpath, "config", buf, len); + VIR_FREE(buf); } else { /* If there's no config data in the virpcitestdata dir, create a dummy * config file */ - make_file(devpath, "config", "some dummy config"); + make_file(devpath, "config", "some dummy config", -1); } if (snprintf(tmp, sizeof(tmp), "0x%.4x", dev->vendor) < 0) ABORT("@tmp overflow"); - make_file(devpath, "vendor", tmp); + make_file(devpath, "vendor", tmp, -1); if (snprintf(tmp, sizeof(tmp), "0x%.4x", dev->device) < 0) ABORT("@tmp overflow"); - make_file(devpath, "device", tmp); + make_file(devpath, "device", tmp, -1); if (pci_device_autobind(dev) < 0) ABORT("Unable to bind: %s", data->id); @@ -348,6 +358,7 @@ pci_device_new_from_stub(const struct pciDevice *data) ABORT_OOM(); VIR_FREE(devpath); + VIR_FREE(configSrc); } static struct pciDevice * @@ -425,10 +436,10 @@ pci_driver_new(const char *name, ...) va_end(args); - make_file(driverpath, "bind", NULL); - make_file(driverpath, "unbind", NULL); - make_file(driverpath, "new_id", NULL); - make_file(driverpath, "remove_id", NULL); + make_file(driverpath, "bind", NULL, -1); + make_file(driverpath, "unbind", NULL, -1); + make_file(driverpath, "new_id", NULL, -1); + make_file(driverpath, "remove_id", NULL, -1); if (VIR_APPEND_ELEMENT_QUIET(pciDrivers, nPciDrivers, driver) < 0) ABORT_OOM(); -- 1.8.3.1

1 0

[libvirt] [PATCH] tests: guarantee abs_srcdir in all C tests
by Eric Blake 27 Nov '13

27 Nov '13

While trying to debug a failure of virpcitest during 'make distcheck', I noticed that with a VPATH build, 'cd tests; ./virpcitest' fails for an entirely different reason. To reproduce the distcheck failure, I had to run 'cd tests; abs_srcdir=/path/to/src ./virpcitest'. But we document in HACKING that all of our tests are supposed to be runnable without requiring extra environment variables. The solution: hardcode the location of srcdir into the just-built binaries, rather than requiring make to prepopulate environment variables. With this, './virpcitest' passes even in a VPATH build (provided that $(srcdir) is writable; a followup patch will fix the conditions required by 'make distcheck'). * tests/Makefile.am (AM_CFLAGS): Define abs_srcdir in all compiled tests. * tests/testutils.h (abs_srcdir): Quit declaring. * tests/testutils.c (virtTestMain): Rely on define rather than environment variable. * tests/virpcimock.c (pci_device_new_from_stub): Rely on define. * tests/cputest.c (mymain): Adjust abs_top_srcdir default. * tests/qemuxml2argvtest.c (mymain): Likewise. * tests/qemuxmlnstest.c (mymain): Likewise. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- Pushing under the build-breaker rule. tests/Makefile.am | 1 + tests/cputest.c | 2 +- tests/qemuxml2argvtest.c | 2 +- tests/qemuxmlnstest.c | 2 +- tests/testutils.c | 11 +---------- tests/testutils.h | 6 +++++- tests/virpcimock.c | 12 ++++-------- 7 files changed, 14 insertions(+), 22 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index e46d5f7..520fd2a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -29,6 +29,7 @@ INCLUDES = \ AM_CFLAGS = \ -Dabs_builddir="\"`pwd`\"" \ + -Dabs_srcdir="\"`cd '$(srcdir)'; pwd`\"" \ $(LIBXML_CFLAGS) \ $(GNUTLS_CFLAGS) \ $(SASL_CFLAGS) \ diff --git a/tests/cputest.c b/tests/cputest.c index b80fac7..20bc544 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -503,7 +503,7 @@ mymain(void) abs_top_srcdir = getenv("abs_top_srcdir"); if (!abs_top_srcdir) - abs_top_srcdir = ".."; + abs_top_srcdir = abs_srcdir "/.."; if (virAsprintf(&map, "%s/src/cpu/cpu_map.xml", abs_top_srcdir) < 0 || cpuMapOverride(map) < 0) { diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index a290062..dad5973 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -297,7 +297,7 @@ mymain(void) abs_top_srcdir = getenv("abs_top_srcdir"); if (!abs_top_srcdir) - abs_top_srcdir = ".."; + abs_top_srcdir = abs_srcdir "/.."; driver.config = virQEMUDriverConfigNew(false); VIR_FREE(driver.config->spiceListen); diff --git a/tests/qemuxmlnstest.c b/tests/qemuxmlnstest.c index 2cc15d1..2093e1e 100644 --- a/tests/qemuxmlnstest.c +++ b/tests/qemuxmlnstest.c @@ -203,7 +203,7 @@ mymain(void) abs_top_srcdir = getenv("abs_top_srcdir"); if (!abs_top_srcdir) - abs_top_srcdir = ".."; + abs_top_srcdir = abs_srcdir "/.."; driver.config = virQEMUDriverConfigNew(false); if ((driver.caps = testQemuCapsInit()) == NULL) diff --git a/tests/testutils.c b/tests/testutils.c index 5d634b4..32fe374 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -69,7 +69,6 @@ static size_t testStart = 0; static size_t testEnd = 0; char *progname; -char *abs_srcdir; void virtTestResult(const char *name, int ret, const char *msg, ...) { @@ -535,15 +534,9 @@ int virtTestMain(int argc, int (*func)(void)) { int ret; - bool abs_srcdir_cleanup = false; char *testRange = NULL; - abs_srcdir = getenv("abs_srcdir"); - if (!abs_srcdir) { - abs_srcdir = getcwd(NULL, 0); - abs_srcdir_cleanup = true; - } - if (!abs_srcdir) + if (!virFileExists(abs_srcdir)) return EXIT_AM_HARDFAIL; progname = last_component(argv[0]); @@ -599,8 +592,6 @@ int virtTestMain(int argc, ret = (func)(); - if (abs_srcdir_cleanup) - VIR_FREE(abs_srcdir); virResetLastError(); if (!virTestGetVerbose() && ret != EXIT_AM_SKIP) { if (testCounter == 0 || testCounter % 40) diff --git a/tests/testutils.h b/tests/testutils.h index 478b53c..674d3df 100644 --- a/tests/testutils.h +++ b/tests/testutils.h @@ -38,7 +38,11 @@ # endif extern char *progname; -extern char *abs_srcdir; + +/* Makefile.am provides these two definitions */ +# if !defined(abs_srcdir) || !defined(abs_builddir) +# error Fix Makefile.am +# endif void virtTestResult(const char *name, int ret, const char *msg, ...) ATTRIBUTE_FMT_PRINTF(3,4); diff --git a/tests/virpcimock.c b/tests/virpcimock.c index 19062c3..48d5f16 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -305,14 +305,10 @@ pci_device_new_from_stub(const struct pciDevice *data) char *configSrc, *configDst; char tmp[32]; struct stat sb; - char *abs_srcdir; - - abs_srcdir = getenv("abs_srcdir"); - if (!abs_srcdir) - abs_srcdir = getcwd(NULL, 0); if (VIR_ALLOC_QUIET(dev) < 0 || - virAsprintfQuiet(&configSrc, "%s/virpcitestdata/%s.config", abs_srcdir, data->id) < 0 || + virAsprintfQuiet(&configSrc, "%s/virpcitestdata/%s.config", + abs_srcdir, data->id) < 0 || virAsprintfQuiet(&devpath, "%s/devices/%s", fakesysfsdir, data->id) < 0) ABORT_OOM(); @@ -480,7 +476,7 @@ pci_driver_bind(struct pciDriver *driver, char *devpath = NULL, *driverpath = NULL; if (dev->driver) { - /* Device already binded */ + /* Device already bound */ errno = ENODEV; return ret; } @@ -527,7 +523,7 @@ pci_driver_unbind(struct pciDriver *driver, char *devpath = NULL, *driverpath = NULL; if (dev->driver != driver) { - /* Device not binded to the @driver */ + /* Device not bound to the @driver */ errno = ENODEV; return ret; } -- 1.8.3.1

1 1

[libvirt] [PATCH] Remove invalid parsing of pty path from XML for PTY type chardevs
by Nehal J Wani 27 Nov '13

27 Nov '13

While running valgrind on the qemuhotplugtest, one of the many memory leaks detected are: ==3915== 12 bytes in 1 blocks are definitely lost in loss record 36 of 129 ==3915== at 0x4A0887C: malloc (vg_replace_malloc.c:270) ==3915== by 0x34268AF275: xmlStrndup (in /usr/lib64/libxml2.so.2.9.1) ==3915== by 0x4CCC3BB: virDomainChrSourceDefParseXML (domain_conf.c:6957) ==3915== by 0x4CD6A29: virDomainChrDefParseXML (domain_conf.c:7243) ==3915== by 0x4CEC0C9: virDomainDeviceDefParse (domain_conf.c:9616) ==3915== by 0x41D14F: testQemuHotplug (qemuhotplugtest.c:247) ==3915== by 0x41E421: virtTestRun (testutils.c:139) ==3915== by 0x41C6E3: mymain (qemuhotplugtest.c:428) ==3915== by 0x41EAB2: virtTestMain (testutils.c:600) ==3915== by 0x341F421A04: (below main) (libc-start.c:225) ==3915== The main reason behind this error is that for the PTY type chardevs, it is not valid to specify or parse a pty path from the XML. This is an output-only attribute, not under user control. Hence, the parsing code inside qemuMonitorJSONAttachCharDev() was wrong. Refer the discussion: https://www.redhat.com/archives/libvir-list/2013-November/msg01255.html tests/qemumonitorjsontest.c * Remove unwanted parsing code inside qemuMonitorJSONAttachCharDev src/qemu/qemu_monitor_json.c: * Remove erraneous testcase --- src/qemu/qemu_monitor_json.c | 20 -------------------- tests/qemumonitorjsontest.c | 16 ---------------- 2 files changed, 36 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index ec3b958..1bfeef0 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5368,26 +5368,6 @@ qemuMonitorJSONAttachCharDev(qemuMonitorPtr mon, if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup; - if (chr->type == VIR_DOMAIN_CHR_TYPE_PTY) { - virJSONValuePtr data; - const char *path; - - if (!(data = virJSONValueObjectGet(reply, "return"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("chardev-add reply was missing return data")); - goto cleanup; - } - - if (!(path = virJSONValueObjectGetString(data, "pty"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("chardev-add reply was missing pty path")); - goto cleanup; - } - - if (VIR_STRDUP(chr->data.file.path, path) < 0) - goto cleanup; - } - ret = 0; cleanup: diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 2152e4a..a3c38b6 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -708,22 +708,6 @@ testQemuMonitorJSONAttachChardev(const void *data) chr = (virDomainChrSourceDef) { .type =VIR_DOMAIN_CHR_TYPE_VC }; CHECK("chr_vc", "{\"return\": {}}"); -#define PTY_PATH "/dev/ttyS0" - chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_PTY }; - CHECK("chr_pty", "{\"return\": {\"pty\" : \"" PTY_PATH "\"}}"); - if (STRNEQ_NULLABLE(PTY_PATH, chr.data.file.path)) { - VIR_FREE(chr.data.file.path); - virReportError(VIR_ERR_INTERNAL_ERROR, - "expected PTY path: %s got: %s", - PTY_PATH, NULLSTR(chr.data.file.path)); - ret = -1; - } - VIR_FREE(chr.data.file.path); - - chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_PTY }; - CHECK_FAIL("chr_pty_fail", "{\"return\": {}}"); -#undef PTY_PATH - chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_FILE }; CHECK("chr_file", "{\"return\": {}}"); -- 1.8.1.4

2 2

[libvirt] [PATCH] Memory Leak Fix: Check def->info->alias before assigning
by Nehal J Wani 27 Nov '13

27 Nov '13

On running the command make -C tests valgrind, there used to be a bunch of memory leaks shown by valgrind. Specifically, one can check it by running: libtool --mode=execute valgrind --quiet --leak-check=full --suppressions=./.valgrind.supp qemuhotplugtest The issue was that def->info->alias was already malloc'ed by xmlStrndup in virDomainDeviceInfoParseXML (domain_conf.c:3439). The new alias was being assigned again without freeing the old one in qemuAssignDeviceAliases(). This patch checks if the entity exists, and frees accordingly, hence making valgrind cry lesser. --- src/qemu/qemu_command.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 763417f..bbec1d4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -979,6 +979,8 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) size_t i; for (i = 0; i < def->ndisks; i++) { + if (def->disks[i]->info.alias) + VIR_FREE(def->disks[i]->info.alias); if (qemuAssignDeviceDiskAlias(def, def->disks[i], qemuCaps) < 0) return -1; } @@ -988,6 +990,9 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) /* type='hostdev' interfaces are also on the hostdevs list, * and will have their alias assigned with other hostdevs. */ + if (def->nets[i]->info.alias) + VIR_FREE(def->nets[i]->info.alias); + if (virDomainNetGetActualType(def->nets[i]) != VIR_DOMAIN_NET_TYPE_HOSTDEV && qemuAssignDeviceNetAlias(def, def->nets[i], i) < 0) { @@ -1000,70 +1005,104 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) return 0; for (i = 0; i < def->nfss; i++) { + if (def->fss[i]->info.alias) + VIR_FREE(def->fss[i]->info.alias); if (virAsprintf(&def->fss[i]->info.alias, "fs%zu", i) < 0) return -1; } for (i = 0; i < def->nsounds; i++) { + if (def->sounds[i]->info.alias) + VIR_FREE(def->sounds[i]->info.alias); if (virAsprintf(&def->sounds[i]->info.alias, "sound%zu", i) < 0) return -1; } for (i = 0; i < def->nhostdevs; i++) { + if (def->hostdevs[i]->info->alias) + VIR_FREE(def->hostdevs[i]->info->alias); if (qemuAssignDeviceHostdevAlias(def, def->hostdevs[i], i) < 0) return -1; } for (i = 0; i < def->nredirdevs; i++) { + if (def->redirdevs[i]->info.alias) + VIR_FREE(def->redirdevs[i]->info.alias); if (qemuAssignDeviceRedirdevAlias(def, def->redirdevs[i], i) < 0) return -1; } for (i = 0; i < def->nvideos; i++) { + if (def->videos[i]->info.alias) + VIR_FREE(def->videos[i]->info.alias); if (virAsprintf(&def->videos[i]->info.alias, "video%zu", i) < 0) return -1; } for (i = 0; i < def->ncontrollers; i++) { + if (def->controllers[i]->info.alias) + VIR_FREE(def->controllers[i]->info.alias); if (qemuAssignDeviceControllerAlias(def->controllers[i]) < 0) return -1; } for (i = 0; i < def->ninputs; i++) { + if (def->inputs[i]->info.alias) + VIR_FREE(def->inputs[i]->info.alias); if (virAsprintf(&def->inputs[i]->info.alias, "input%zu", i) < 0) return -1; } for (i = 0; i < def->nparallels; i++) { + if (def->parallels[i]->info.alias) + VIR_FREE(def->parallels[i]->info.alias); if (qemuAssignDeviceChrAlias(def, def->parallels[i], i) < 0) return -1; } for (i = 0; i < def->nserials; i++) { + if (def->serials[i]->info.alias) + VIR_FREE(def->serials[i]->info.alias); if (qemuAssignDeviceChrAlias(def, def->serials[i], i) < 0) return -1; } for (i = 0; i < def->nchannels; i++) { + if (def->channels[i]->info.alias) + VIR_FREE(def->channels[i]->info.alias); if (qemuAssignDeviceChrAlias(def, def->channels[i], i) < 0) return -1; } for (i = 0; i < def->nconsoles; i++) { + if (def->consoles[i]->info.alias) + VIR_FREE(def->consoles[i]->info.alias); if (qemuAssignDeviceChrAlias(def, def->consoles[i], i) < 0) return -1; } for (i = 0; i < def->nhubs; i++) { + if (def->hubs[i]->info.alias) + VIR_FREE(def->hubs[i]->info.alias); if (virAsprintf(&def->hubs[i]->info.alias, "hub%zu", i) < 0) return -1; } for (i = 0; i < def->nsmartcards; i++) { + if (def->smartcards[i]->info.alias) + VIR_FREE(def->smartcards[i]->info.alias); if (virAsprintf(&def->smartcards[i]->info.alias, "smartcard%zu", i) < 0) return -1; } if (def->watchdog) { + if (def->watchdog->info.alias) + VIR_FREE(def->watchdog->info.alias); if (virAsprintf(&def->watchdog->info.alias, "watchdog%d", 0) < 0) return -1; } if (def->memballoon) { + if (def->memballoon->info.alias) + VIR_FREE(def->memballoon->info.alias); if (virAsprintf(&def->memballoon->info.alias, "balloon%d", 0) < 0) return -1; } if (def->rng) { + if (def->rng->info.alias) + VIR_FREE(def->rng->info.alias); if (virAsprintf(&def->rng->info.alias, "rng%d", 0) < 0) return -1; } if (def->tpm) { + if (def->tpm->info.alias) + VIR_FREE(def->tpm->info.alias); if (virAsprintf(&def->tpm->info.alias, "tpm%d", 0) < 0) return -1; } -- 1.8.1.4

4 7

[libvirt] [PATCH python] Deal with old filenames for events/error functions
by Daniel P. Berrange 27 Nov '13

27 Nov '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> Older libvirt has files named 'events' and 'virterror' rather than 'virevent' and 'virerror'. This is visible in the API XML files. We must look for both names to ensure we don't loose generation of methods with older versions of libvirt. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- generator.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/generator.py b/generator.py index 273efbd..cd857b4 100755 --- a/generator.py +++ b/generator.py @@ -113,7 +113,8 @@ class docParser(xml.sax.handler.ContentHandler): elif tag == 'enum': # enums come from header files, hence virterror.h if (attrs['file'] == "libvirt" or - attrs['file'] == "virterror"): + attrs['file'] == "virterror" or + attrs['file'] == "virerror"): enum(attrs['type'],attrs['name'],attrs['value']) elif attrs['file'] == "libvirt-lxc": lxc_enum(attrs['type'],attrs['name'],attrs['value']) @@ -127,8 +128,10 @@ class docParser(xml.sax.handler.ContentHandler): # fuctions come from source files, hence 'virerror.c' if self.function is not None: if (self.function_module == "libvirt" or + self.function_module == "event" or self.function_module == "virevent" or - self.function_module == "virerror"): + self.function_module == "virerror" or + self.function_module == "virterror"): function(self.function, self.function_descr, self.function_return, self.function_args, self.function_file, self.function_module, -- 1.8.3.1

3 2

[libvirt] [PATCH] storage: skip selinux cleanup when fd not available
by Eric Blake 27 Nov '13

27 Nov '13

When attempting to backport gluster pools to an older version where there is no VIR_STRDUP, I got a crash from calling strdup(,NULL). Rather than relying on the current else branch safely doing nothing when there is no fd, it is easier to just skip it. While at it, there's no need to explicitly set perms.label to NULL after a VIR_FREE(). * src/storage/storage_backend.c (virStorageBackendUpdateVolTargetInfoFD): Minor optimization. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- src/storage/storage_backend.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index bde39d6..b08d646 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1383,28 +1383,26 @@ virStorageBackendUpdateVolTargetInfoFD(virStorageVolTargetPtr target, VIR_FREE(target->perms.label); #if WITH_SELINUX /* XXX: make this a security driver call */ - if (fd >= 0 && fgetfilecon_raw(fd, &filecon) == -1) { - if (errno != ENODATA && errno != ENOTSUP) { - virReportSystemError(errno, - _("cannot get file context of '%s'"), - target->path); - return -1; + if (fd >= 0) { + if (fgetfilecon_raw(fd, &filecon) == -1) { + if (errno != ENODATA && errno != ENOTSUP) { + virReportSystemError(errno, + _("cannot get file context of '%s'"), + target->path); + return -1; + } } else { - target->perms.label = NULL; - } - } else { - if (VIR_STRDUP(target->perms.label, filecon) < 0) { + if (VIR_STRDUP(target->perms.label, filecon) < 0) { + freecon(filecon); + return -1; + } freecon(filecon); - return -1; } - freecon(filecon); } -#else - target->perms.label = NULL; #endif return 0; } -- 1.8.3.1

2 2

[libvirt] [PATCH python] Fix misc RPM specfile flaws
by Daniel P. Berrange 27 Nov '13

27 Nov '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> Fix the RPM summary line, add placeholder %changelog tag, make %setup quiet, add Url: tag and filter out bogus provides and add example programs as docs. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- libvirt-python.spec.in | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/libvirt-python.spec.in b/libvirt-python.spec.in index e6c229f..7c6257e 100644 --- a/libvirt-python.spec.in +++ b/libvirt-python.spec.in @@ -1,14 +1,19 @@ -Summary: The libvirt virtualization API +Summary: The libvirt virtualization API python binding Name: libvirt-python Version: @PY_VERSION@ Release: 1%{?dist}%{?extra_release} Source0: http://libvirt.org/sources/python/%{name}-%{version}.tar.gz +Url: http://libvirt.org License: LGPLv2+ Group: Development/Libraries BuildRequires: libvirt-devel >= @C_VERSION@ BuildRequires: python-devel +# Don't want provides for python shared objects +%{?filter_provides_in: %filter_provides_in %{python_sitearch}/.*\.so} +%{?filter_setup} + %description The libvirt-python package contains a module that permits applications written in the Python programming language to use the interface @@ -16,7 +21,7 @@ supplied by the libvirt library to use the virtualization capabilities of recent versions of Linux (and other OSes). %prep -%setup +%setup -q %build CFLAGS="$RPM_OPT_FLAGS" %{__python} setup.py build @@ -27,8 +32,10 @@ rm -f %{buildroot}%{_libdir}/python*/site-packages/*egg-info %files %defattr(-,root,root) -%doc ChangeLog AUTHORS NEWS README COPYING COPYING.LESSER +%doc ChangeLog AUTHORS NEWS README COPYING COPYING.LESSER examples/ %{_libdir}/python*/site-packages/libvirt.py* %{_libdir}/python*/site-packages/libvirt_qemu.py* %{_libdir}/python*/site-packages/libvirt_lxc.py* %{_libdir}/python*/site-packages/libvirtmod* + +%changelog -- 1.8.3.1

2 1

[libvirt] [PATCH python] Fix code for avoiding overrides of non-existant functions
by Daniel P. Berrange 27 Nov '13

27 Nov '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> When reading/writing a global variable from inside a method it must be declared as a global, otherwise a local variable by the same name will be used. Special case the virConnectListDomainsID method which is bizarrely renamed for no obvious reason. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- generator.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/generator.py b/generator.py index cd857b4..e8d8ea9 100755 --- a/generator.py +++ b/generator.py @@ -187,16 +187,21 @@ class docParser(xml.sax.handler.ContentHandler): def function(name, desc, ret, args, file, module, cond): + global onlyOverrides if onlyOverrides and name not in functions: return + if name == "virConnectListDomains": + name = "virConnectListDomainsID" functions[name] = (desc, ret, args, file, module, cond) def qemu_function(name, desc, ret, args, file, module, cond): + global onlyOverrides if onlyOverrides and name not in qemu_functions: return qemu_functions[name] = (desc, ret, args, file, module, cond) def lxc_function(name, desc, ret, args, file, module, cond): + global onlyOverrides if onlyOverrides and name not in lxc_functions: return lxc_functions[name] = (desc, ret, args, file, module, cond) @@ -786,6 +791,7 @@ def buildStubs(module, api_xml): global py_types global py_return_types global unknown_types + global onlyOverrides if module not in ["libvirt", "libvirt-qemu", "libvirt-lxc"]: print "ERROR: Unknown module type: %s" % module -- 1.8.3.1

2 1

[libvirt] [PATCH] Make virsh command 'domxml-to-native' copy the MAC addr parsed from XML
by mars＠linux.vnet.ibm.com 27 Nov '13

27 Nov '13

From: Bing Bu Cao <mars(a)linux.vnet.ibm.com> virsh command 'domxml-to-native' did not copy the MAC address which parsed from a domain xml, in the commandline the MAC addr of network device always was 00:00:00:00:00:00. This patch fix it. Signed-off-by: Bing Bu Cao <mars(a)linux.vnet.ibm.com> --- src/qemu/qemu_driver.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8a1eefd..4693dad 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5821,6 +5821,7 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, virDomainNetDefPtr net = def->nets[i]; int bootIndex = net->info.bootIndex; char *model = net->model; + virMacAddr mac = net->mac; if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { int actualType = virDomainNetGetActualType(net); @@ -5843,6 +5844,7 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, net->script = NULL; net->data.ethernet.dev = brnamecopy; net->data.ethernet.ipaddr = NULL; + net->mac = mac; } else { /* actualType is either NETWORK or DIRECT. In either * case, the best we can do is NULL everything out. @@ -5854,6 +5856,7 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, net->script = NULL; net->data.ethernet.dev = NULL; net->data.ethernet.ipaddr = NULL; + net->mac = mac; } } else if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) { VIR_FREE(net->data.direct.linkdev); @@ -5864,6 +5867,7 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, net->script = NULL; net->data.ethernet.dev = NULL; net->data.ethernet.ipaddr = NULL; + net->mac = mac; } else if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) { char *script = net->script; char *brname = net->data.bridge.brname; @@ -5875,6 +5879,7 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, net->script = script; net->data.ethernet.dev = brname; net->data.ethernet.ipaddr = ipaddr; + net->mac = mac; } VIR_FREE(net->virtPortProfile); -- 1.7.7.6

2 1

[libvirt] [PATCH python] Add missing binding of security model/label APIs
by Daniel P. Berrange 27 Nov '13

27 Nov '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> The virNodeGetSecurityModel, virDomainGetSecurityLabel and virDomainGetSecurityLabelList methods were disabled in the python binding for inexplicable reasons. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- generator.py | 6 ++-- libvirt-override-api.xml | 15 +++++++++ libvirt-override.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++ typewrappers.c | 9 ++++++ typewrappers.h | 1 + 5 files changed, 110 insertions(+), 3 deletions(-) diff --git a/generator.py b/generator.py index ab2a97f..273efbd 100755 --- a/generator.py +++ b/generator.py @@ -381,6 +381,9 @@ skip_impl = ( 'virDomainGetJobInfo', 'virDomainGetJobStats', 'virNodeGetInfo', + 'virNodeGetSecurityModel', + 'virDomainGetSecurityLabel', + 'virDomainGetSecurityLabelList', 'virDomainGetUUID', 'virDomainGetUUIDString', 'virDomainLookupByUUID', @@ -476,9 +479,6 @@ skip_function = ( 'virCopyLastError', # Python API is called virGetLastError instead 'virConnectOpenAuth', # Python C code is manually written 'virDefaultErrorFunc', # Python virErrorFuncHandler impl calls this from C - 'virDomainGetSecurityLabel', # Needs investigation... - 'virDomainGetSecurityLabelList', # Needs investigation... - 'virNodeGetSecurityModel', # Needs investigation... 'virConnectDomainEventRegister', # overridden in virConnect.py 'virConnectDomainEventDeregister', # overridden in virConnect.py 'virConnectDomainEventRegisterAny', # overridden in virConnect.py diff --git a/libvirt-override-api.xml b/libvirt-override-api.xml index 337d0a0..d5b25b5 100644 --- a/libvirt-override-api.xml +++ b/libvirt-override-api.xml @@ -104,6 +104,21 @@ <return type='char *' info='the list of information or None in case of error'/> <arg name='conn' type='virConnectPtr' info='pointer to the hypervisor connection'/> </function> + <function name='virNodeGetSecurityModel' file='python'> + <info>Extract information about the host security model</info> + <return type='char *' info='the list of information or None in case of error'/> + <arg name='conn' type='virConnectPtr' info='pointer to the hypervisor connection'/> + </function> + <function name='virDomainGetSecurityLabel' file='python'> + <info>Extract information about the domain security label. Only the first label will be returned.</info> + <return type='char *' info='the list of information or None in case of error'/> + <arg name='domain' type='virDomainPtr' info='a domain object'/> + </function> + <function name='virDomainGetSecurityLabelList' file='python'> + <info>Extract information about the domain security label. A list of all labels will be returned.</info> + <return type='char *' info='the list of information or None in case of error'/> + <arg name='domain' type='virDomainPtr' info='a domain object'/> + </function> <function name='virNodeGetCPUStats' file='python'> <info>Extract node's CPU statistics.</info> <return type='char *' info='dictionary mapping field names to values or None in case of error'/> diff --git a/libvirt-override.c b/libvirt-override.c index d3802de..42e253e 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -2865,6 +2865,83 @@ libvirt_virNodeGetInfo(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { } static PyObject * +libvirt_virNodeGetSecurityModel(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { + PyObject *py_retval; + int c_retval; + virConnectPtr conn; + PyObject *pyobj_conn; + virSecurityModel model; + + if (!PyArg_ParseTuple(args, (char *)"O:virDomainGetSecurityModel", &pyobj_conn)) + return NULL; + conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn); + + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virNodeGetSecurityModel(conn, &model); + LIBVIRT_END_ALLOW_THREADS; + if (c_retval < 0) + return VIR_PY_NONE; + py_retval = PyList_New(2); + PyList_SetItem(py_retval, 0, libvirt_constcharPtrWrap(&model.model[0])); + PyList_SetItem(py_retval, 1, libvirt_constcharPtrWrap(&model.doi[0])); + return py_retval; +} + +static PyObject * +libvirt_virDomainGetSecurityLabel(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { + PyObject *py_retval; + int c_retval; + virDomainPtr dom; + PyObject *pyobj_dom; + virSecurityLabel label; + + if (!PyArg_ParseTuple(args, (char *)"O:virDomainGetSecurityLabel", &pyobj_dom)) + return NULL; + dom = (virDomainPtr) PyvirDomain_Get(pyobj_dom); + + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virDomainGetSecurityLabel(dom, &label); + LIBVIRT_END_ALLOW_THREADS; + if (c_retval < 0) + return VIR_PY_NONE; + py_retval = PyList_New(2); + PyList_SetItem(py_retval, 0, libvirt_constcharPtrWrap(&label.label[0])); + PyList_SetItem(py_retval, 1, libvirt_boolWrap(label.enforcing)); + return py_retval; +} + +#if LIBVIR_CHECK_VERSION(0, 9, 13) +static PyObject * +libvirt_virDomainGetSecurityLabelList(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { + PyObject *py_retval; + int c_retval; + virDomainPtr dom; + PyObject *pyobj_dom; + virSecurityLabel *labels; + size_t i; + + if (!PyArg_ParseTuple(args, (char *)"O:virDomainGetSecurityLabel", &pyobj_dom)) + return NULL; + dom = (virDomainPtr) PyvirDomain_Get(pyobj_dom); + + LIBVIRT_BEGIN_ALLOW_THREADS; + c_retval = virDomainGetSecurityLabelList(dom, &labels); + LIBVIRT_END_ALLOW_THREADS; + if (c_retval < 0) + return VIR_PY_NONE; + py_retval = PyList_New(0); + for (i = 0 ; i < c_retval ; i++) { + PyObject *entry = PyList_New(2); + PyList_SetItem(entry, 0, libvirt_constcharPtrWrap(&labels[i].label[0])); + PyList_SetItem(entry, 1, libvirt_boolWrap(labels[i].enforcing)); + PyList_Append(py_retval, entry); + } + free(labels); + return py_retval; +} +#endif + +static PyObject * libvirt_virDomainGetUUID(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { PyObject *py_retval; unsigned char uuid[VIR_UUID_BUFLEN]; @@ -7304,6 +7381,11 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virDomainGetControlInfo", libvirt_virDomainGetControlInfo, METH_VARARGS, NULL}, {(char *) "virDomainGetBlockInfo", libvirt_virDomainGetBlockInfo, METH_VARARGS, NULL}, {(char *) "virNodeGetInfo", libvirt_virNodeGetInfo, METH_VARARGS, NULL}, + {(char *) "virNodeGetSecurityModel", libvirt_virNodeGetSecurityModel, METH_VARARGS, NULL}, + {(char *) "virDomainGetSecurityLabel", libvirt_virDomainGetSecurityLabel, METH_VARARGS, NULL}, +#if LIBVIR_CHECK_VERSION(0, 9, 13) + {(char *) "virDomainGetSecurityLabelList", libvirt_virDomainGetSecurityLabelList, METH_VARARGS, NULL}, +#endif /* LIBVIR_CHECK_VERSION(0, 9, 13) */ {(char *) "virNodeGetCPUStats", libvirt_virNodeGetCPUStats, METH_VARARGS, NULL}, {(char *) "virNodeGetMemoryStats", libvirt_virNodeGetMemoryStats, METH_VARARGS, NULL}, {(char *) "virDomainGetUUID", libvirt_virDomainGetUUID, METH_VARARGS, NULL}, diff --git a/typewrappers.c b/typewrappers.c index d6cbbf1..1622986 100644 --- a/typewrappers.c +++ b/typewrappers.c @@ -116,6 +116,15 @@ libvirt_constcharPtrWrap(const char *str) return ret; } +PyObject * +libvirt_boolWrap(int val) +{ + if (val) + Py_RETURN_TRUE; + else + Py_RETURN_FALSE; +} + int libvirt_intUnwrap(PyObject *obj, int *val) { diff --git a/typewrappers.h b/typewrappers.h index d871d3f..04e364f 100644 --- a/typewrappers.h +++ b/typewrappers.h @@ -164,6 +164,7 @@ PyObject * libvirt_ulonglongWrap(unsigned long long val); PyObject * libvirt_charPtrWrap(char *str); PyObject * libvirt_charPtrSizeWrap(char *str, Py_ssize_t size); PyObject * libvirt_constcharPtrWrap(const char *str); +PyObject * libvirt_boolWrap(int val); int libvirt_intUnwrap(PyObject *obj, int *val); int libvirt_uintUnwrap(PyObject *obj, unsigned int *val); int libvirt_longUnwrap(PyObject *obj, long *val); -- 1.8.3.1

2 2

[libvirt] [PATCH python] Avoid generating the migrate methods in multiple classes
by Daniel P. Berrange 27 Nov '13

27 Nov '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> The python code generator tries to figure out what class a method should be in by looking at the list of arguments for any which are object types. Unfortunately missing break statements meant that methods which have multiple object arguments (eg migrate as a virDomainPtr followed by a virConnectPtr) got added to multiple classes. The migrate methods should only be visible in the virDomain class, and the versions in the virConnect class have fubar arguments. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- generator.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/generator.py b/generator.py index c769ed0..ab2a97f 100755 --- a/generator.py +++ b/generator.py @@ -1264,12 +1264,14 @@ def buildWrappers(module): func = nameFixup(name, classe, type, file) info = (0, func, name, ret, args, file, mod) function_classes[classe].append(info) + break elif name[0:3] == "vir" and len(args) >= 2 and args[1][1] == type \ and file != "python_accessor" and not name in function_skip_index_one: found = 1 func = nameFixup(name, classe, type, file) info = (1, func, name, ret, args, file, mod) function_classes[classe].append(info) + break if found == 1: continue func = nameFixup(name, "None", file, file) -- 1.8.3.1

2 2

[libvirt] virDomain{Attach, Detach, Update}DeviceFlags filesystem support for qemu driver
by Jesse Cook 27 Nov '13

27 Nov '13

It does not currently appear to be possible to attach/detach a filesystem with kvm through the C API or virsh. Looking at src/qemu/qemu_driver.c:qemuDomain{Attach,Detach,Update}Config, VIR_DOMAIN_DEVICE_FS is not a case within the switch statement for dev->type. Is support for this planned for the near future or in progress? If not, how much would have to be implemented outside these functions to add support for attaching, detaching, and modifying filesystems? I would like to gauge whether I should carve out time to add support, or work around it by using disks in place of filesystems. Thanks! -- Jesse J. Cook Camber Corporation Lead Software Developer

2 1

[libvirt] [libvirt-test-API][PATCH] Fix utils.exec_cmd output problem
by Jincheng Miao 27 Nov '13

27 Nov '13

The 'out' returned from exec_cmds is not 'None' type if stderr occurs, it is because utils pass 'subprocess.PIPE' to stdout to open this subprocess. "Similarly, to get anything other than None in the result tuple, you need to give stdout=PIPE and/or stderr=PIPE too." (see http://docs.python.org/2/library/subprocess.html#module-subprocess) Finally, make out equals to err when stderr happened. --- utils/utils.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/utils/utils.py b/utils/utils.py index 147c1ef..2248ce1 100644 --- a/utils/utils.py +++ b/utils/utils.py @@ -404,14 +404,12 @@ def exec_cmd(command, sudo=False, cwd=None, infile=None, outfile=None, shell=Fal command = ["sudo"] + command if infile == None: infile = subprocess.PIPE - if outfile == None: - outfile = subprocess.PIPE p = subprocess.Popen(command, shell=shell, close_fds=True, cwd=cwd, stdin=infile, stdout=outfile, stderr=subprocess.PIPE) (out, err) = p.communicate(data) if out == None: - # Prevent splitlines() from barfing later on - out = "" + # Because stderr is PIPE, err will not be None, and can be splitlines. + out = err return (p.returncode, out.splitlines()) def remote_exec_pexpect(hostname, username, password, cmd): -- 1.8.3.1

2 8

[libvirt] [PATCH] look for numad in /usr/sbin
by Jim Fehlig 27 Nov '13

27 Nov '13

When looking for numad with AC_PATH_PROG, include /usr/sbin in the search path. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 4942e07..f494f46 100644 --- a/configure.ac +++ b/configure.ac @@ -1454,7 +1454,7 @@ AC_ARG_WITH([numad], if test "$with_numad" != "no" ; then fail=0 - AC_PATH_PROG([NUMAD], [numad], [], [/bin:/usr/bin]) + AC_PATH_PROG([NUMAD], [numad], [], [/bin:/usr/bin:/usr/sbin]) if test "$with_numad" = "check"; then test "$with_numactl" = "yes" || fail=1 -- 1.8.0.1

2 2