[libvirt] [PATCH v2] virObject: Error on suspicious ref and unref

2013-11-28 Thread Michal Privoznik
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

2013-11-28 Thread Weiqing Wang
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

2013-11-28 Thread Umar Draz
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

2013-11-28 Thread WeiqingGmail
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

2013-11-28 Thread Jiri Denemark
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

2013-11-28 Thread Michal Privoznik
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

2013-11-28 Thread Daniel P. Berrange
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

2013-11-28 Thread Michal Privoznik
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

2013-11-28 Thread Laine Stump
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

2013-11-28 Thread Nehal J Wani
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

2013-11-28 Thread 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

Re: [libvirt] [PATCH 1/3] add percentage limit parse and define support for RAM filesystems

2013-11-28 Thread Chen Hanxiao


> -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

2013-11-28 Thread Chen Hanxiao
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

2013-11-28 Thread Daniel P. Berrange
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

2013-11-28 Thread Daniel P. Berrange
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

2013-11-28 Thread Daniel P. Berrange
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

2013-11-28 Thread Daniel P. Berrange
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

2013-11-28 Thread Chen Hanxiao
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

2013-11-28 Thread Chen Hanxiao
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

2013-11-28 Thread Chen Hanxiao
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

2013-11-28 Thread Chen Hanxiao
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

2013-11-28 Thread Osier Yang

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

2013-11-28 Thread Osier Yang

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"

2013-11-28 Thread Osier Yang

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

2013-11-28 Thread Osier Yang

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

2013-11-28 Thread Osier Yang

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

2013-11-28 Thread Hu Tao
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

2013-11-28 Thread Hu Tao
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

2013-11-28 Thread Osier Yang

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