[libvirt] [PATCH v2] virObject: Error on suspicious ref and unref
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 0x7f496ff44607 in ncf_close (ncf=0x7f496801e3d0) at netcf.c:101 __PRETTY_FUNCTION__ = "ncf_close" #5 0x7f4984f3f31b in virObjectUnref (anyobj=) at util/virobject.c:262 klass = 0x7f4968019020 obj = 0x7f4968018260 lastRef = true __func__ = "virObjectUnref" #6 0x7f4970159785 in netcfStateCleanup () at interface/interface_backend_netcf.c:109 No locals. #7 0x7f4984fc0e28 in virStateCleanup () at libvirt.c:883 i = 6 ret = 0 #8 0x7f49859b55fd in main (argc=, argv=) at libvirtd.c:1549 srv = 0x7f498696ed00 remote_config_file = 0x0 statuswrite = -1 ret = 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 = implicit_conf = run_dir = 0x0 old_umask = 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 0x7f496ff44607 in ncf_close (ncf=0x7f496801e3d0) at netcf.c:101 __PRETTY_FUNCTION__ = "ncf_close" #7 0x7f4984f3f31b in virObjectUnref (anyobj=) at util/virobject.c:262 klass = 0x7f4968019020 obj = 0x7f4968018260 lastRef = true __func__ = "virObjectUnref" #8 0x7f4970157752 in netcfInterfaceClose (conn=0x7f49680a3260) at interface/interface_backend_netcf.c:265 No locals. #9 0x7f4984fb7984 in virConnectDispose (obj=0x7f49680a3260) at datatypes.c:149 conn = 0x7f49680a3260 #10 0x7f4984f3f31b in virObjectUnref (anyobj=anyobj@entry=0x7f49680a3260) at util/virobject.c:262 klass = 0x7f49681d6760 obj = 0x7f49680a3260 lastRef = true __func__ = "virObjectUnref" #11 0x7f4984fc11bf in virConnectClose (conn=conn@entry=0x7f49680a3260) at libvirt.c:1532 __func__ = "virConnectClose" __FUNCTION__ = "virConnectClose" #12 0x7f496eced281 in virLXCProcessAutostartAll (driver=0x7f49681ffaa0) at lxc/lxc_process.c:1426 conn = 0x7f49680a3260 data = {driver = 0x7f49681ffaa0, conn = 0x7f49680a3260} #13 0x7f4984fc0d21 in virStateInitialize (privileged=true, callback=callback@entry=0x7f49859b7180 , opaque=opaque@entry=0x7f498696ed00) at libvirt.c:864 i = 8 __func__ = "virStateInitialize" #14 0x7f49859b71db in daemonRunStateInit (opaque=opaque@entry=0x7f498696ed00) at libvirtd.c:906 srv = 0x7f498696ed00 sysident = 0x7f4968000ae0 __func__ = "daemonRunStateInit" #15 0x7f4984f4efe1 in virThreadHelper (data=) at util/virthreadpthread.c:161 args = 0x0 local = {func = 0x7f49859b71a0 , opaque = 0x7f498696ed00} #16 0x7f4982a40de3 in start_thread (arg=0x7f496d6a4700) at pthread_create.c:308 __res = pd = 0x7f496d6a4700 now = 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 = pagesize_m1 = sp = freesize = #17 0x7f498236716d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:113 Signed-off-by: Michal Privoznik --- 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
Re: [libvirt] GlusterFS with libvirt
Hi Umar Just a week ago, I had tested redhat storage, which is based on glusterfs.In the guide that redhat provide, the glusterfs volume transport is asked to be TCP. I don't know why this document say you can define the transport type as socket or rdma or unix. About the question you said in the first email, I can't give you any answer. Sorry! Regards Weiqing On 2013-11-29, at 11:22, Umar Draz wrote: > HI Weiqing, > > Yes I want storage pool of glusterfs. > > http://www.gluster.org/community/documentation/images/9/9d/QEMU_GlusterFS.pdf > > > Br. > > Umar > > > On Fri, Nov 29, 2013 at 6:48 AM, WeiqingGmail wrote: >> Hi Umar >> >> > Is any body help me how I can integrate libvirt with glusterfs on Ubuntu >> > 13.10 Server. >> >> Glusterfs is a solution for cloud storage. >> I don't get your point about integrating libvirt with Glusterfs. >> Are you want to build guest in the storage pool that consist of Glusterfs >> Volume? >> >> Regards >> Weiqing >> 在 2013-11-28,20:55,Umar Draz 写道: >> >> > Hi All, >> > >> > Is any body help me how I can integrate libvirt with glusterfs on Ubuntu >> > 13.10 Server. >> > >> > I tried the following, but its not working. >> > >> > qemu-img: Unknown protocol 'gluster://localhost/gv1/test.img' >> > >> > >> > Br. >> > >> > Umar >> > -- >> > libvir-list mailing list >> > libvir-list@redhat.com >> > https://www.redhat.com/mailman/listinfo/libvir-list > > > > -- > Umar Draz > Network Architect -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] GlusterFS with libvirt
HI Weiqing, Yes I want storage pool of glusterfs. http://www.gluster.org/community/documentation/images/9/9d/QEMU_GlusterFS.pdf Br. Umar On Fri, Nov 29, 2013 at 6:48 AM, WeiqingGmail wrote: > Hi Umar > > > Is any body help me how I can integrate libvirt with glusterfs on Ubuntu > 13.10 Server. > > Glusterfs is a solution for cloud storage. > I don't get your point about integrating libvirt with Glusterfs. > Are you want to build guest in the storage pool that consist of Glusterfs > Volume? > > Regards > Weiqing > 在 2013-11-28,20:55,Umar Draz 写道: > > > Hi All, > > > > Is any body help me how I can integrate libvirt with glusterfs on Ubuntu > 13.10 Server. > > > > I tried the following, but its not working. > > > > qemu-img: Unknown protocol 'gluster://localhost/gv1/test.img' > > > > > > Br. > > > > Umar > > -- > > libvir-list mailing list > > libvir-list@redhat.com > > https://www.redhat.com/mailman/listinfo/libvir-list > -- Umar Draz Network Architect -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] GlusterFS with libvirt
Hi Umar > Is any body help me how I can integrate libvirt with glusterfs on Ubuntu > 13.10 Server. Glusterfs is a solution for cloud storage. I don't get your point about integrating libvirt with Glusterfs. Are you want to build guest in the storage pool that consist of Glusterfs Volume? Regards Weiqing 在 2013-11-28,20:55,Umar Draz 写道: > Hi All, > > Is any body help me how I can integrate libvirt with glusterfs on Ubuntu > 13.10 Server. > > I tried the following, but its not working. > > qemu-img: Unknown protocol 'gluster://localhost/gv1/test.img' > > > Br. > > Umar > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virObject: Error on suspicious ref and unref
On Thu, Nov 28, 2013 at 17:06:09 +0100, Michal Privoznik wrote: > 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): ... > 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; } I believe you didn't want to copy&paste from virAtomicIntDecAndTest without modifications :-). This one is an atomic version of { *atomic -= 1; return *atomic; } > + * > + * This call acts as a full compiler and hardware memory barrier. > + */ > +VIR_STATIC int virAtomicIntDec(volatile int *atomic) > +ATTRIBUTE_NONNULL(1); > + > +/** ... Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virObject: Error on suspicious ref and unref
On 28.11.2013 17:19, Daniel P. Berrange wrote: > On Thu, Nov 28, 2013 at 05:06:09PM +0100, Michal Privoznik wrote: >> 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 0x7f496ff44607 in ncf_close (ncf=0x7f496801e3d0) at netcf.c:101 >> __PRETTY_FUNCTION__ = "ncf_close" >> #5 0x7f4984f3f31b in virObjectUnref (anyobj=) at >> util/virobject.c:262 >> klass = 0x7f4968019020 >> obj = 0x7f4968018260 >> lastRef = true >> __func__ = "virObjectUnref" >> #6 0x7f4970159785 in netcfStateCleanup () at >> interface/interface_backend_netcf.c:109 >> No locals. >> #7 0x7f4984fc0e28 in virStateCleanup () at libvirt.c:883 >> i = 6 >> ret = 0 >> #8 0x7f49859b55fd in main (argc=, argv=) >> at libvirtd.c:1549 >> srv = 0x7f498696ed00 >> remote_config_file = 0x0 >> statuswrite = -1 >> ret = >> 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 = >> implicit_conf = >> run_dir = 0x0 >> old_umask = >> 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 0x7f496ff44607 in ncf_close (ncf=0x7f496801e3d0) at netcf.c:101 >> __PRETTY_FUNCTION__ = "ncf_close" >> #7 0x7f4984f3f31b in virObjectUnref (anyobj=) at >> util/virobject.c:262 >> klass = 0x7f4968019020 >> obj = 0x7f4968018260 >> lastRef = true >> __func__ = "virObjectUnref" >> #8 0x7f4970157752 in netcfInterfaceClose (conn=0x7f49680a3260) at >> interface/interface_backend_netcf.c:265 >> No locals. >> #9 0x7f4984fb7984 in virConnectDispose (obj=0x7f49680a3260) at >> datatypes.c:149 >> conn = 0x7f49680a3260 >> #10 0x7f4984f3f31b in virObjectUnref >> (anyobj=anyobj@entry=0x7f49680a3260) at util/virobject.c:262 >> klass = 0x7f49681d6760 >> obj = 0x7f49680a3260 >> lastRef = true >> __func__ = "virObjectUnref" >> #11 0x7f4984fc11bf in virConnectClose (conn=conn@entry=0x7f49680a3260) >> at libvirt.c:1532 >> __func__ = "virConnectClose" >> __FUNCTION__ = "virConnectClose" >> #12 0x7f496eced281 in virLXCProcessAutostartAll (driver=0x7f49681ffaa0) >> at lxc/lxc_process.c:1426 >> conn = 0x7f49680a3260 >> data = {driver = 0x7f49681ffaa0, conn = 0x7f49680a3260} >> #13 0x7f4984fc0d21 in virStateInitialize (privileged=true, >> callback=callback@entry=0x7f49859b7180 , >> opaque=opaque@entry=0x7f498696ed00) at libvirt.c:864 >> i = 8 >> __func__ = "virStateInitialize" >> #14 0x7f49859b71db in daemonRunStateInit >> (opaque=opaque@entry=0x7f498696ed00) at libvirtd.c:906 >> srv = 0x7f498696ed00 >> sysident = 0x7f4968000ae0 >> __func__ = "daemonRunStateInit" >> #15 0x7f4984f4efe1 in virThreadHelper (data=) at >> util/virthreadpthread.c:161 >> args = 0x0 >> local = {func = 0x7f49859b71a0 , opaque = >> 0x7f498696ed00} >> #16 0x7f4982a40de3 in start_thread (arg=0x7f496d6a4700) at >> pthread_create.c:308 >> __res = >> pd = 0x7f496d6a4700 >> now = >> 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
Re: [libvirt] [PATCH] virObject: Error on suspicious ref and unref
On Thu, Nov 28, 2013 at 05:06:09PM +0100, Michal Privoznik wrote: > 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 0x7f496ff44607 in ncf_close (ncf=0x7f496801e3d0) at netcf.c:101 > __PRETTY_FUNCTION__ = "ncf_close" > #5 0x7f4984f3f31b in virObjectUnref (anyobj=) at > util/virobject.c:262 > klass = 0x7f4968019020 > obj = 0x7f4968018260 > lastRef = true > __func__ = "virObjectUnref" > #6 0x7f4970159785 in netcfStateCleanup () at > interface/interface_backend_netcf.c:109 > No locals. > #7 0x7f4984fc0e28 in virStateCleanup () at libvirt.c:883 > i = 6 > ret = 0 > #8 0x7f49859b55fd in main (argc=, argv=) > at libvirtd.c:1549 > srv = 0x7f498696ed00 > remote_config_file = 0x0 > statuswrite = -1 > ret = > 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 = > implicit_conf = > run_dir = 0x0 > old_umask = > 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 0x7f496ff44607 in ncf_close (ncf=0x7f496801e3d0) at netcf.c:101 > __PRETTY_FUNCTION__ = "ncf_close" > #7 0x7f4984f3f31b in virObjectUnref (anyobj=) at > util/virobject.c:262 > klass = 0x7f4968019020 > obj = 0x7f4968018260 > lastRef = true > __func__ = "virObjectUnref" > #8 0x7f4970157752 in netcfInterfaceClose (conn=0x7f49680a3260) at > interface/interface_backend_netcf.c:265 > No locals. > #9 0x7f4984fb7984 in virConnectDispose (obj=0x7f49680a3260) at > datatypes.c:149 > conn = 0x7f49680a3260 > #10 0x7f4984f3f31b in virObjectUnref > (anyobj=anyobj@entry=0x7f49680a3260) at util/virobject.c:262 > klass = 0x7f49681d6760 > obj = 0x7f49680a3260 > lastRef = true > __func__ = "virObjectUnref" > #11 0x7f4984fc11bf in virConnectClose (conn=conn@entry=0x7f49680a3260) > at libvirt.c:1532 > __func__ = "virConnectClose" > __FUNCTION__ = "virConnectClose" > #12 0x7f496eced281 in virLXCProcessAutostartAll (driver=0x7f49681ffaa0) > at lxc/lxc_process.c:1426 > conn = 0x7f49680a3260 > data = {driver = 0x7f49681ffaa0, conn = 0x7f49680a3260} > #13 0x7f4984fc0d21 in virStateInitialize (privileged=true, > callback=callback@entry=0x7f49859b7180 , > opaque=opaque@entry=0x7f498696ed00) at libvirt.c:864 > i = 8 > __func__ = "virStateInitialize" > #14 0x7f49859b71db in daemonRunStateInit > (opaque=opaque@entry=0x7f498696ed00) at libvirtd.c:906 > srv = 0x7f498696ed00 > sysident = 0x7f4968000ae0 > __func__ = "daemonRunStateInit" > #15 0x7f4984f4efe1 in virThreadHelper (data=) at > util/virthreadpthread.c:161 > args = 0x0 > local = {func = 0x7f49859b71a0 , opaque = > 0x7f498696ed00} > #16 0x7f4982a40de3 in start_thread (arg=0x7f496d6a4700) at > pthread_create.c:308 > __res = > pd = 0x7f496d6a4700 > now = > 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 = > pagesize_m1 = > sp = > freesize = > #17 0x7f498236716d in clone () at > ../sysdeps/unix/sys
[libvirt] [PATCH] virObject: Error on suspicious ref and unref
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 0x7f496ff44607 in ncf_close (ncf=0x7f496801e3d0) at netcf.c:101 __PRETTY_FUNCTION__ = "ncf_close" #5 0x7f4984f3f31b in virObjectUnref (anyobj=) at util/virobject.c:262 klass = 0x7f4968019020 obj = 0x7f4968018260 lastRef = true __func__ = "virObjectUnref" #6 0x7f4970159785 in netcfStateCleanup () at interface/interface_backend_netcf.c:109 No locals. #7 0x7f4984fc0e28 in virStateCleanup () at libvirt.c:883 i = 6 ret = 0 #8 0x7f49859b55fd in main (argc=, argv=) at libvirtd.c:1549 srv = 0x7f498696ed00 remote_config_file = 0x0 statuswrite = -1 ret = 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 = implicit_conf = run_dir = 0x0 old_umask = 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 0x7f496ff44607 in ncf_close (ncf=0x7f496801e3d0) at netcf.c:101 __PRETTY_FUNCTION__ = "ncf_close" #7 0x7f4984f3f31b in virObjectUnref (anyobj=) at util/virobject.c:262 klass = 0x7f4968019020 obj = 0x7f4968018260 lastRef = true __func__ = "virObjectUnref" #8 0x7f4970157752 in netcfInterfaceClose (conn=0x7f49680a3260) at interface/interface_backend_netcf.c:265 No locals. #9 0x7f4984fb7984 in virConnectDispose (obj=0x7f49680a3260) at datatypes.c:149 conn = 0x7f49680a3260 #10 0x7f4984f3f31b in virObjectUnref (anyobj=anyobj@entry=0x7f49680a3260) at util/virobject.c:262 klass = 0x7f49681d6760 obj = 0x7f49680a3260 lastRef = true __func__ = "virObjectUnref" #11 0x7f4984fc11bf in virConnectClose (conn=conn@entry=0x7f49680a3260) at libvirt.c:1532 __func__ = "virConnectClose" __FUNCTION__ = "virConnectClose" #12 0x7f496eced281 in virLXCProcessAutostartAll (driver=0x7f49681ffaa0) at lxc/lxc_process.c:1426 conn = 0x7f49680a3260 data = {driver = 0x7f49681ffaa0, conn = 0x7f49680a3260} #13 0x7f4984fc0d21 in virStateInitialize (privileged=true, callback=callback@entry=0x7f49859b7180 , opaque=opaque@entry=0x7f498696ed00) at libvirt.c:864 i = 8 __func__ = "virStateInitialize" #14 0x7f49859b71db in daemonRunStateInit (opaque=opaque@entry=0x7f498696ed00) at libvirtd.c:906 srv = 0x7f498696ed00 sysident = 0x7f4968000ae0 __func__ = "daemonRunStateInit" #15 0x7f4984f4efe1 in virThreadHelper (data=) at util/virthreadpthread.c:161 args = 0x0 local = {func = 0x7f49859b71a0 , opaque = 0x7f498696ed00} #16 0x7f4982a40de3 in start_thread (arg=0x7f496d6a4700) at pthread_create.c:308 __res = pd = 0x7f496d6a4700 now = 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 = pagesize_m1 = sp = freesize = #17 0x7f498236716d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:113 Signed-off-by: Michal Privoznik --- src/nwfilter/nwfilter_dhcpsnoop.c | 6 ++--- src/util/viratomic.h | 53 +++ src/util/virobject.c | 17 ++--- 3 files changed, 58 insertions(+), 18 deletions(-) diff
Re: [libvirt] What to do about the qemu "-boot strict" option
On 11/28/2013 01:56 AM, Amos Kong wrote: > On Wed, Nov 27, 2013 at 02:37:02PM +0200, Laine Stump wrote: >> 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. > We have a reboot-timeout boot parameter to reboot guest if not found > bootable device. Yes. libvirt supports that parameter. The problem was that the disk was "bootable", but just happened to boot into an "operating system" whose only function was to print out BOOT DISK FAILURE, PRESS ANY KEY then wait indefinitely for a key to be pressed :-) >> 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). > We leave the default as off just for compatibility with old qemu. > For libvirt code, you can always use "strict=on" Right. The fact that it's configurable in qemu raises the question of whether there may be legitimate cases for libvirt where someone would expect/demand the old behavior, and the new behavior would cause breakage of existing setups. If there are, I don't want to prevent them by making it unconfigurable, but if there aren't, I don't want to clutter up config with yet another unnecessary knob. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: fix typos in libvirt.h.in
On Thu, Nov 28, 2013 at 5:31 PM, Chen Hanxiao wrote: > From: Chen Hanxiao > > s/caused/cause > > Signed-off-by: Chen Hanxiao > --- > include/libvirt/libvirt.h.in | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in > index 146a59b..934d425 100644 > --- a/include/libvirt/libvirt.h.in > +++ b/include/libvirt/libvirt.h.in > @@ -3461,7 +3461,7 @@ typedef enum { > /** > * virDomainEventDefinedDetailType: > * > - * Details on the caused of the 'defined' lifecycle event > + * Details on the cause of the 'defined' lifecycle event > */ > 1.8.2.1 > I am no English expert, but would like to ask, if a particular event can take place due to multiple reasons, shouldn't it be : "Details on the causes of the 'defined' lifecycle event" I think the original author made a typo by typing 'd' instead of 's' (very close on keyboard), i.e., s/caused/causes -- Nehal J Wani -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] GlusterFS with libvirt
Hi All, Is any body help me how I can integrate libvirt with glusterfs on Ubuntu 13.10 Server. I tried the following, but its not working. qemu-img: Unknown protocol 'gluster://localhost/gv1/test.img' Br. Umar -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] add percentage limit parse and define support for RAM filesystems
> -Original Message- > From: Daniel P. Berrange [mailto:berra...@redhat.com] > Sent: Thursday, November 28, 2013 6:35 PM > To: Chen Hanxiao > Cc: libvir-list@redhat.com > Subject: Re: [libvirt] [PATCH 1/3] add percentage limit parse and define > support > for RAM filesystems > > On Thu, Nov 28, 2013 at 05:14:43PM +0800, Chen Hanxiao wrote: > > From: Chen Hanxiao > > > > This patch enables percentage limit for ram filesystem > > > > > > > > > > > > > > Percentage limit would have more priority than size limit. > > > > Signed-off-by: Chen Hanxiao > > --- > > src/conf/domain_conf.c | 21 + > > src/conf/domain_conf.h | 1 + > > 2 files changed, 18 insertions(+), 4 deletions(-) > > I'm not really convinced we need this feature. Seems like more code for > little real benefit. > I think we should follow the style of mount(8). It accepted this style. And this feature could bring us convenience in config, free us from counting the size. Thanks. > Regards, > Daniel > -- > |: http://berrange.com -o- > http://www.flickr.com/photos/dberrange/ :| > |: http://libvirt.org -o- > http://virt-manager.org :| > |: http://autobuild.org -o- > http://search.cpan.org/~danberr/ :| > |: http://entangle-photo.org -o- > http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] docs: fix typos in libvirt.h.in
From: Chen Hanxiao s/caused/cause Signed-off-by: Chen Hanxiao --- include/libvirt/libvirt.h.in | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 146a59b..934d425 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3461,7 +3461,7 @@ typedef enum { /** * virDomainEventDefinedDetailType: * - * Details on the caused of the 'defined' lifecycle event + * Details on the cause of the 'defined' lifecycle event */ typedef enum { VIR_DOMAIN_EVENT_DEFINED_ADDED = 0, /* Newly created config file */ @@ -3475,7 +3475,7 @@ typedef enum { /** * virDomainEventUndefinedDetailType: * - * Details on the caused of the 'undefined' lifecycle event + * Details on the cause of the 'undefined' lifecycle event */ typedef enum { VIR_DOMAIN_EVENT_UNDEFINED_REMOVED = 0, /* Deleted the config file */ @@ -3488,7 +3488,7 @@ typedef enum { /** * virDomainEventStartedDetailType: * - * Details on the caused of the 'started' lifecycle event + * Details on the cause of the 'started' lifecycle event */ typedef enum { VIR_DOMAIN_EVENT_STARTED_BOOTED = 0, /* Normal startup from boot */ @@ -3505,7 +3505,7 @@ typedef enum { /** * virDomainEventSuspendedDetailType: * - * Details on the caused of the 'suspended' lifecycle event + * Details on the cause of the 'suspended' lifecycle event */ typedef enum { VIR_DOMAIN_EVENT_SUSPENDED_PAUSED = 0, /* Normal suspend due to admin pause */ @@ -3524,7 +3524,7 @@ typedef enum { /** * virDomainEventResumedDetailType: * - * Details on the caused of the 'resumed' lifecycle event + * Details on the cause of the 'resumed' lifecycle event */ typedef enum { VIR_DOMAIN_EVENT_RESUMED_UNPAUSED = 0, /* Normal resume due to admin unpause */ @@ -3539,7 +3539,7 @@ typedef enum { /** * virDomainEventStoppedDetailType: * - * Details on the caused of the 'stopped' lifecycle event + * Details on the cause of the 'stopped' lifecycle event */ typedef enum { VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN = 0, /* Normal shutdown */ -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH python] Improve quality of sanitytest check
On Wed, Nov 27, 2013 at 10:28:08AM -0500, Doug Goldstein wrote: > > > > On Nov 27, 2013, at 6:19 AM, "Daniel P. Berrange" > > wrote: > > > >> On Tue, Nov 26, 2013 at 01:16:24PM -0600, Doug Goldstein wrote: > >> On Tue, Nov 26, 2013 at 12:32 PM, Daniel P. Berrange > >> wrote: > >>> From: "Daniel P. Berrange" > >>> > >>> Validate that every public API method is mapped into the python > >>> and that every python method has a sane C API. > >> > >> Looks like we had the same idea and even a similar approach as well. > >> > >>> > >>> Signed-off-by: Daniel P. Berrange > >>> --- > >>> 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 > > Can we drop the explicit lxml import since we are only using > etree? Then we can try lxml.etree and xml.etree as fallbacks. > Do we need lxml added to the spec file for building as well? The 'xpath' function is lxml only - the regular etree module only has 'find' which doesn't do proper xpath queries. > >>> +# 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) > >>> + Daniel. -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] Pull lxcContainerGetSubtree out into shared virfile module
On Wed, Nov 27, 2013 at 03:25:28PM -0700, Eric Blake wrote: > On 11/27/2013 09:31 AM, Daniel P. Berrange wrote: > > From: "Daniel P. Berrange" > > > > Move the code for lxcContainerGetSubtree into the virfile > > module creating 2 new functions > > > > int virFileGetMountSubtree(const char *mtabpath, > > const char *prefix, > > char ***mountsret, > > size_t *nmountsret); > > int virFileGetMountReverseSubtree(const char *mtabpath, > > const char *prefix, > > char ***mountsret, > > size_t *nmountsret); > > > > Add a new virfiletest.c test case to validate the new code. > > > > Signed-off-by: Daniel P. Berrange > > --- > > > @@ -1747,6 +1749,8 @@ virStringArrayHasString; > > virStringFreeList; > > virStringJoin; > > virStringListLength; > > +virStringSortCompare; > > +virStringSortRevCompare; > > Maybe worth splitting this into an independent patch, as virsh also > could use it. Actually virsh can't use it - it sorts virDomainPtr instances rather than just the name strings. That said I've split it off anyway, and added a test case to virstringtest.c Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] add percentage limit parse and define support for RAM filesystems
On Thu, Nov 28, 2013 at 05:14:43PM +0800, Chen Hanxiao wrote: > From: Chen Hanxiao > > This patch enables percentage limit for ram filesystem > > > > > > > Percentage limit would have more priority than size limit. > > Signed-off-by: Chen Hanxiao > --- > src/conf/domain_conf.c | 21 + > src/conf/domain_conf.h | 1 + > 2 files changed, 18 insertions(+), 4 deletions(-) I'm not really convinced we need this feature. Seems like more code for little real benefit. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: add support for -device pvpanic
On Thu, Nov 28, 2013 at 04:09:00PM +0800, Hu Tao wrote: > On Wed, Nov 27, 2013 at 06:15:27AM -0700, Eric Blake wrote: > > On 11/27/2013 03:39 AM, Peter Krempa wrote: > > > On 11/27/13 09:41, Hu Tao wrote: > > >> qemu removes the builtin pvpanic device for all qemu versions since 1.7, > > >> in order to support , '-device pvpanic' has to be added to > > >> qemu command line. > > >> > > >> Signed-off-by: Hu Tao > > >> --- > > > > > I remember discussions saying that it's NOT a good idea to enable this > > > stuff always. As a result, this device is not being added by qemu as you > > > described above. Shouldn't we only add this if the user enables > > > actions? > > > > You are precisely right; we MUST add a new entry under in the > > XML before enabling this device. > > Is a entry under for pvpanic still needed? What I thought is > that it is natural to enable pvpanic when user enables , > he/she even has no need to know about pvpanic. No, the elements are *soley* about setting policy. They must never have any impact on hardware visibility. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] docs: add docs for percentage limit
From: Chen Hanxiao Signed-off-by: Chen Hanxiao --- docs/formatdomain.html.in | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 096e2a3..ac330cb 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2233,7 +2233,9 @@ An in-memory filesystem, using memory from the host OS. The source element has a single attribute usage which gives the memory usage limit in KiB, unless units - are specified by the units attribute. Only used + are specified by the units attribute. + Attribute usage with a suffix % will gives + the memory usage limit by percentage.Only used by LXC driver. (since 0.9.13) type='bind' -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] add percentage limit parse and define support for RAM filesystems
From: Chen Hanxiao This patch enables percentage limit for ram filesystem Percentage limit would have more priority than size limit. Signed-off-by: Chen Hanxiao --- src/conf/domain_conf.c | 21 + src/conf/domain_conf.h | 1 + 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d3f1ca2..25cfde2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6068,19 +6068,27 @@ virDomainFSDefParseXML(xmlNodePtr node, } if (def->type == VIR_DOMAIN_FS_TYPE_RAM) { +int len; if (!usage) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing 'usage' attribute for RAM filesystem")); goto error; } +len = strlen(usage); +if ('%' == usage[len - 1]) { +VIR_DEBUG("usage is %s\n", usage); +*(usage + len - 1) = '\0'; +def->usage_percentage = true; +} + if (virStrToLong_ull(usage, NULL, 10, &def->usage) < 0) { virReportError(VIR_ERR_XML_ERROR, _("cannot parse usage '%s' for RAM filesystem"), usage); goto error; } -if (virScaleInteger(&def->usage, units, -1024, ULLONG_MAX) < 0) +if (!(def->usage_percentage) && +virScaleInteger(&def->usage, units, 1024, ULLONG_MAX) < 0) goto error; } @@ -14866,8 +14874,13 @@ virDomainFSDefFormat(virBufferPtr buf, break; case VIR_DOMAIN_FS_TYPE_RAM: -virBufferAsprintf(buf, " \n", - def->usage / 1024); +if (def->usage_percentage) { +virBufferAsprintf(buf, " \n", + def->usage); +} else { +virBufferAsprintf(buf, " \n", + def->usage / 1024); +} break; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4561ccc..abfa756 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -891,6 +891,7 @@ struct _virDomainFSDef { int wrpolicy; /* enum virDomainFSWrpolicy */ int format; /* enum virStorageFileFormat */ unsigned long long usage; /* in bytes */ +bool usage_percentage; /* flag for percentage limit */ char *src; char *dst; bool readonly; -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] add percentage limit support for RAM filesystems in LXC container
From: Chen Hanxiao Chen Hanxiao (3): add percentage limit parse and define support for RAM filesystems enable percentage limit of RAM filesystem in lxc container docs: add docs for percentage limitation docs/formatdomain.html.in | 4 +++- src/conf/domain_conf.c| 21 + src/conf/domain_conf.h| 1 + src/lxc/lxc_container.c | 14 +++--- 4 files changed, 32 insertions(+), 8 deletions(-) -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] enable percentage limit of RAM filesystem in lxc container
From: Chen Hanxiao Signed-off-by: Chen Hanxiao --- src/lxc/lxc_container.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 2bdf957..408499a 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1447,9 +1447,17 @@ static int lxcContainerMountFSTmpfs(virDomainFSDefPtr fs, VIR_DEBUG("usage=%lld sec=%s", fs->usage, sec_mount_options); -if (virAsprintf(&data, -"size=%lld%s", fs->usage, sec_mount_options) < 0) -goto cleanup; +if (fs->usage_percentage) { +VIR_DEBUG("use percentage limit for tmpfs"); +if (virAsprintf(&data, +"size=%lld%%%s", fs->usage, sec_mount_options) < 0) +goto cleanup; +} else { +VIR_DEBUG("use size limit for tmpfs"); +if (virAsprintf(&data, +"size=%lld%s", fs->usage, sec_mount_options) < 0) +goto cleanup; +} if (virFileMakePath(fs->dst) < 0) { virReportSystemError(errno, -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 4/4] qemu: Refactor qemuTranslatePool source
On 27/11/13 23:14, Peter Krempa wrote: Before this patch, the translation function still needs a second ugly helper function to actually format the command line for qemu. But if we do the right stuff in the translation functio, we don't have to bother s/functio/function/, with the second function any more. This patch removes the messy qemuBuildVolumeString() function and changes qemuTranslatePool() to set stuff up correctly so that the s/qemuTranslatePool/qemuTranslateDiskSourcePool/, regular code paths meant for volumes can be used to format the command line correctly. For this purpose a new helper "qemuDiskGetActualType()" is introduced to return the type of the volume in a pool. As a part of the refactor the qemuTranslatePool function is fixed to do Same as above. decisions based on the pool type instead of the volume type. This allows to separate pool-type-specific stuff more clearly and will ease addition of other pool types that will require certain other operations to get the correct pool source. The previously fixed tests should make sure that we don't break stuff that was working before. --- src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 76 +++- src/qemu/qemu_conf.c | 129 --- src/qemu/qemu_conf.h | 2 + 5 files changed, 98 insertions(+), 111 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4561ccc..a5ef2ca 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -686,6 +686,7 @@ struct _virDomainDiskSourcePoolDef { char *volume; /* volume name */ int voltype; /* enum virStorageVolType, internal only */ int pooltype; /* enum virStoragePoolType, internal only */ +int actualtype; /* enum virDomainDiskType, internal only */ int mode; /* enum virDomainDiskSourcePoolMode */ }; typedef virDomainDiskSourcePoolDef *virDomainDiskSourcePoolDefPtr; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 205fe56..aeb3568 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -693,6 +693,7 @@ virStoragePoolSourceFree; virStoragePoolSourceListFormat; virStoragePoolSourceListNewSource; virStoragePoolTypeFromString; +virStoragePoolTypeToString; virStorageVolDefFindByKey; virStorageVolDefFindByName; virStorageVolDefFindByPath; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 763417f..b8129a3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3775,69 +3775,6 @@ qemuBuildNBDString(virConnectPtr conn, virDomainDiskDefPtr disk, virBufferPtr op return 0; } -static int -qemuBuildVolumeString(virConnectPtr conn, - virDomainDiskDefPtr disk, - virBufferPtr opt) I admit I was confused when reviewing v2 without the commit log. -{ -int ret = -1; - -switch ((virStorageVolType) disk->srcpool->voltype) { -case VIR_STORAGE_VOL_DIR: -if (!disk->readonly) { -virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot create virtual FAT disks in read-write mode")); -goto cleanup; -} -if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) -virBufferEscape(opt, ',', ",", "file=fat:floppy:%s,", disk->src); -else -virBufferEscape(opt, ',', ",", "file=fat:%s,", disk->src); -break; -case VIR_STORAGE_VOL_BLOCK: -if (disk->tray_status == VIR_DOMAIN_DISK_TRAY_OPEN) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("tray status 'open' is invalid for " - "block type volume")); [1] I keep my opinion that it should provide an exact error, either "block" or "volume" is the term we used in the XML and documents. One will be confused to see "tray status 'open' is invalid for block type disk", since it's actually defined as type="volume". It doesn't have to be the same, but there should be a way to differentiate them. -goto cleanup; -} -if (disk->srcpool->pooltype == VIR_STORAGE_POOL_ISCSI) { -if (disk->srcpool->mode == -VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT) { -if (qemuBuildISCSIString(conn, disk, opt) < 0) -goto cleanup; -virBufferAddChar(opt, ','); -} else if (disk->srcpool->mode == - VIR_DOMAIN_DISK_SOURCE_POOL_MODE_HOST) { -virBufferEscape(opt, ',', ",", "file=%s,", disk->src); -} -} else { -virBufferEscape(opt, ',', ",", "file=%s,", disk->src); -} -break; -case VIR_STORAGE_VOL_FILE: -if (disk->auth.username) { -if (qemuBuildISCSIString(conn, disk, opt) < 0) -goto cleanup; -virBufferAddChar(opt, ','); -} else { -
Re: [libvirt] [PATCHv2 3/4] qemuxml2argv: Add test for disk type='volume' with iSCSI pools
On 27/11/13 23:14, Peter Krempa wrote: Tweak the existing file so that it can be tested for command line corectness. --- tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args | 10 ++ tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml | 4 ++-- tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 14 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args new file mode 100644 index 000..8f6a3dd --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.args @@ -0,0 +1,10 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -drive \ +file=/some/block/device/unit:0:0:1,if=none,media=cdrom,id=drive-ide0-0-1 -device \ +ide-drive,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 -drive \ +file=iscsi://iscsi.example.com:3260/demo-target/2,if=none,media=cdrom,id=drive-ide0-0-2 \ +-device ide-drive,bus=ide.0,unit=2,drive=drive-ide0-0-2,id=ide0-0-2 -drive \ +file=/tmp/idedisk.img,if=none,id=drive-ide0-0-3 -device \ +ide-drive,bus=ide.0,unit=3,drive=drive-ide0-0-3,id=ide0-0-3 -device \ +virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml index b907633..9f90293 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool-mode.xml @@ -15,7 +15,7 @@ /usr/bin/qemu - + system_u:system_r:public_content_t:s0 @@ -25,7 +25,7 @@ - + Okay, I see why you removed the "startupPolicy" now, it doesn't make sense for a pool with block type volume. ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 2/4] qemuxml2argv: Add test to verify correct usage of disk type="volume"
On 27/11/13 23:14, Peter Krempa wrote: Tweak the existing file to test command line generator too. --- tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.args | 8 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.xml | 2 +- tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-source-pool.args Michal's ACK stands. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 1/4] test: Implement fake storage pool driver in qemuxml2argv test
On 27/11/13 23:14, Peter Krempa wrote: To support testing of "volume" disk backing, we need to implement a few disk driver backend functions. The fake storage driver uses files in storagepoolxml2xmlout/POOLNAME.xml as XML files for pool definitions and volume names are in format "VOL_TYPE+VOL_PATH". By default type "block" is assumed (for iSCSI test compatibility). The choice of this approach along with implemented functions was made so that can be tested in the xml2argv test. --- tests/qemuxml2argvtest.c | 183 +++ 1 file changed, 183 insertions(+) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index a290062..9c8f8e2 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -18,6 +18,7 @@ # include "qemu/qemu_command.h" # include "qemu/qemu_domain.h" # include "datatypes.h" +# include "conf/storage_conf.h" # include "cpu/cpu_map.h" # include "virstring.h" @@ -75,6 +76,182 @@ static virSecretDriver fakeSecretDriver = { .secretUndefine = NULL, }; + +# define STORAGE_POOL_XML_PATH "storagepoolxml2xmlout/" +static const unsigned char fakeUUID[VIR_UUID_BUFLEN] = "fakeuuid"; + +static virStoragePoolPtr +fakeStoragePoolLookupByName(virConnectPtr conn, +const char *name) +{ +char *xmlpath = NULL; +virStoragePoolPtr ret = NULL; + +if (STRNEQ(name, "inactive")) { +if (virAsprintf(&xmlpath, "%s/%s%s.xml", +abs_srcdir, +STORAGE_POOL_XML_PATH, +name) < 0) Ok, this solves the build problem. +return NULL; + +if (!virFileExists(xmlpath)) { +virReportError(VIR_ERR_NO_STORAGE_POOL, + "File '%s' not found", xmlpath); +goto cleanup; +} +} + +ret = virGetStoragePool(conn, name, fakeUUID, NULL, NULL); + +cleanup: +VIR_FREE(xmlpath); +return ret; +} + + +static virStorageVolPtr +fakeStorageVolLookupByName(virStoragePoolPtr pool, + const char *name) +{ +char **volinfo = NULL; +virStorageVolPtr ret = NULL; + +if (STREQ(pool->name, "inactive")) { +virReportError(VIR_ERR_OPERATION_INVALID, + "storage pool '%s' is not active", pool->name); +return NULL; +} + +if (STREQ(name, "nonexistent")) { +virReportError(VIR_ERR_NO_STORAGE_VOL, + "no storage vol with matching name '%s'", name); +return NULL; +} + +if (!strchr(name, '+')) +goto fallback; + +if (!(volinfo = virStringSplit(name, "+", 2))) +return NULL; + +if (!volinfo[1]) +goto fallback; + +ret = virGetStorageVol(pool->conn, pool->name, volinfo[1], volinfo[0], + NULL, NULL); + +cleanup: +virStringFreeList(volinfo); +return ret; + +fallback: +ret = virGetStorageVol(pool->conn, pool->name, name, "block", NULL, NULL); +goto cleanup; +} + +static int +fakeStorageVolGetInfo(virStorageVolPtr vol, + virStorageVolInfoPtr info) +{ +memset(info, 0, sizeof(*info)); + +info->type = virStorageVolTypeFromString(vol->key); + +if (info->type < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + "Invalid volume type '%s'", vol->key); +return -1; +} + +return 0; +} + + +static char * +fakeStorageVolGetPath(virStorageVolPtr vol) +{ +char *ret = NULL; + +ignore_value(virAsprintf(&ret, "/some/%s/device/%s", vol->key, vol->name)); + +return ret; +} + + +static char * +fakeStoragePoolGetXMLDesc(virStoragePoolPtr pool, + unsigned int flags_unused ATTRIBUTE_UNUSED) +{ +char *xmlpath = NULL; +char *xmlbuf = NULL; + +if (STREQ(pool->name, "inactive")) { +virReportError(VIR_ERR_NO_STORAGE_POOL, NULL); +return NULL; +} + +if (virAsprintf(&xmlpath, "%s/%s%s.xml", +abs_srcdir, +STORAGE_POOL_XML_PATH, +pool->name) < 0) +return NULL; + +if (virtTestLoadFile(xmlpath, &xmlbuf) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + "failed to load XML file '%s'", + xmlpath); +goto cleanup; +} + +cleanup: +VIR_FREE(xmlpath); + +return xmlbuf; +} + +static int +fakeStorageClose(virConnectPtr conn ATTRIBUTE_UNUSED) +{ +return 0; +} + +static int +fakeStoragePoolIsActive(virStoragePoolPtr pool) +{ +if (STREQ(pool->name, "inactive")) +return 0; + +return 1; +} + +/* Test storage pool implementation + * + * These functions aid testing of storage pool related stuff when creating a + * qemu command . s/command \./command line\./, + * + * There are a few "magic" values to pass to these functions: + * + * 1) "inactive" as + * a pool name for pool lookup creates a inactive pool. All other names are s/a
Re: [libvirt] [PATCHv1.5 02/27] test: Implement fake storage pool driver in qemuxml2argv test
On 27/11/13 17:58, Peter Krempa wrote: On 11/27/13 08:47, Osier Yang wrote: On 27/11/13 00:48, Peter Krempa wrote: To support testing of "volume" disk backing, we need to implement a few disk driver backend functions. The fake storage driver uses files in storagepoolxml2xmlout/POOLNAME.xml as XML files for pool definitions and volume names are in format "VOL_TYPE+VOL_PATH". By default type "block" is assumed (for iSCSI test compatibility). The choice of this approach along with implemented functions was made so that can be tested in the xml2argv test. --- tests/qemuxml2argvtest.c | 162 +++ 1 file changed, 162 insertions(+) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index a290062..a4cef84 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -18,6 +18,7 @@ # include "qemu/qemu_command.h" # include "qemu/qemu_domain.h" # include "datatypes.h" +# include "conf/storage_conf.h" # include "cpu/cpu_map.h" # include "virstring.h" @@ -75,6 +76,161 @@ static virSecretDriver fakeSecretDriver = { .secretUndefine = NULL, }; + +# define STORAGE_POOL_XML_PATH "storagepoolxml2xmlout/" This will cause build failure when building with VPATH. Hmmm, I'll look into it. FYI, You can use "abs_srcdir" directly to construct the path now, after Eric's patch is pushed: https://www.redhat.com/archives/libvir-list/2013-November/msg01265.html Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: add support for -device pvpanic
On Wed, Nov 27, 2013 at 06:15:27AM -0700, Eric Blake wrote: > On 11/27/2013 03:39 AM, Peter Krempa wrote: > > On 11/27/13 09:41, Hu Tao wrote: > >> qemu removes the builtin pvpanic device for all qemu versions since 1.7, > >> in order to support , '-device pvpanic' has to be added to > >> qemu command line. > >> > >> Signed-off-by: Hu Tao > >> --- > > > I remember discussions saying that it's NOT a good idea to enable this > > stuff always. As a result, this device is not being added by qemu as you > > described above. Shouldn't we only add this if the user enables > > actions? > > You are precisely right; we MUST add a new entry under in the > XML before enabling this device. Is a entry under for pvpanic still needed? What I thought is that it is natural to enable pvpanic when user enables , he/she even has no need to know about pvpanic. > > See these threads for some ideas (although recall that qemu has been > fixed in the meantime to state that any distro shipping a qemu with > pvpanic enabled by default can be considered buggy, and that libvirt can > now assume that pvpanic will not happen without an explicit '-device > pvpanic'): > https://www.redhat.com/archives/libvir-list/2013-August/msg01184.html > https://www.redhat.com/archives/libvir-list/2013-August/msg01136.html IIRC, at the time of the thread, the pvpanic entry under is for addressing the compatibility of qemu 1.5(has builtin pvpanic) and qemu 1.6 and after(has no builtin pvpanic). Since now qemu removes builtin pvpanic for all versions, I think there is no need for pvpanic entry in xml. -- Regards, Hu Tao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: add support for -device pvpanic
On Wed, Nov 27, 2013 at 11:39:49AM +0100, Peter Krempa wrote: > On 11/27/13 09:41, Hu Tao wrote: > > qemu removes the builtin pvpanic device for all qemu versions since 1.7, > > in order to support , '-device pvpanic' has to be added to > > qemu command line. > > > > Signed-off-by: Hu Tao > > --- > > 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); > > + > > Hard coding the version number is not a good idea. Libvirt uses qmp > monitor queries to determine supported devices. > > Please add this in the virQEMUCapsObjectTypes structure instead, which > will do the qmp detection for you. Thanks for advising, I'll do it in next version. > > > 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); > > + > > Same here ... this should be autoprobed by a function using the > structure above. > > > 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); > > +} > > + > > I remember discussions saying that it's NOT a good idea to enable this > stuff always. As a result, this device is not being added by qemu as you > described above. Shouldn't we only add this if the user enables > actions? Yes we should. When I was writing the patch, I found that def->onCrash has a default value even when user doesn't set . I must be reading the code wrong. Any, let me fix it in next version. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Doc: Explicitly declaring that nodedev-destroy only works for vHBA
On 22/11/13 20:55, Osier Yang wrote: Can someone give an obvious ACK? Though trying to destroy a physical HBA doesn't make sense at all, it's still a bit misleading with saying "only works for HBA". Signed-off-by: Osier Yang --- src/libvirt.c | 5 +++-- tools/virsh.pod | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index ae05300..4ebd13f 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -16065,8 +16065,9 @@ error: * virNodeDeviceDestroy: * @dev: a device object * - * Destroy the device object. The virtual device is removed from the host operating system. - * This function may require privileged access + * Destroy the device object. The virtual device (only works for vHBA + * currently) is removed from the host operating system. This function + * may require privileged access. * * Returns 0 in case of success and -1 in case of failure. */ diff --git a/tools/virsh.pod b/tools/virsh.pod index dac9a08..557df9c 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2158,9 +2158,9 @@ contains xml for a top-level description of a node device. =item B I Destroy (stop) a device on the host. I can be either device -name or wwn pair in "wwnn,wwpn" format (only works for HBA). Note -that this makes libvirt quit managing a host device, and may even make -that device unusable by the rest of the physical host until a reboot. +name or wwn pair in "wwnn,wwpn" format (only works for vHBA currently). +Note that this makes libvirt quit managing a host device, and may even +make that device unusable by the rest of the physical host until a reboot. =item B I [I<--driver backend_driver>] -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list