Re: [libvirt] [PATCH v2 2/2] qemu: completely rework reference counting

2014-12-21 Thread Martin Kletzander

On Tue, Dec 16, 2014 at 04:51:59PM +0100, Martin Kletzander wrote:

..., but I'll wait with the pushing so others have a chance to
react.



There's no reaction for a while now and since I'd like to push this in
the early stage of a release cycle, I'm pushing it now.  This touches
almost all APIs and hence we want some time to fix things nobody
noticed yet.

Martin


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH V3 1/5] Add public API virDomainSendSysrq

2014-12-21 Thread Peter Krempa
On 12/19/14 13:03, John Ferlan wrote:
 
 
 On 12/19/2014 12:31 AM, Chun Yan Liu wrote:


 On 12/18/2014 at 01:00 PM, in message
 5492d008026600086...@soto.provo.novell.com, Chun Yan Liu
 cy...@suse.com wrote:


 On 12/17/2014 at 06:52 PM, in message
 20141217105227.gq136...@orkuz.home,
 Jiri Denemark jdene...@redhat.com wrote:
 On Wed, Dec 17, 2014 at 16:48:52 +0800, Chunyan Liu wrote:
 Add public API virDomainSendSysrq for sending SysRequest key.

 Signed-off-by: Chunyan Liu cy...@suse.com
 ---
 changes:
* add 'flags' to the new API
* change parameter from 'const char *key' to 'char key'
* change version number from 1.2.11 to 1.2.12

   include/libvirt/libvirt-domain.h |  3 +++
   src/driver-hypervisor.h  |  4 
   src/libvirt-domain.c | 39
 +++
   src/libvirt_public.syms  |  5 +
   4 files changed, 51 insertions(+)

 diff --git a/include/libvirt/libvirt-domain.h
 b/include/libvirt/libvirt-domain.h
 index baef32d..5f72850 100644
 --- a/include/libvirt/libvirt-domain.h
 +++ b/include/libvirt/libvirt-domain.h
 @@ -3526,6 +3526,9 @@ int virDomainGetFSInfo(virDomainPtr dom,
  virDomainFSInfoPtr **info,
  unsigned int flags);

 +/* virDomainSendSysrq */
 +int virDomainSendSysrq(virDomainPtr dom, char key, unsigned int
 flags);
 +

 I think quite a few reviewers (Daniel, Eric, and I) agreed on using an
 enum instead of char so that the API is more general.

 Sorry, I missed this part. I'll update. One left question:
 How about 'virsh sysrq' parameters? What would we expect users to pass?

 Any thoughts on that?
   libxl_send_sysrq
 
 Without a virsh.pod in v3 to go with virsh-domain.c, I'm not sure what
 you had in mind for syntax previously - although it looks like:
 
 virsh sysrq domain [key]
 
 Where if not provided key would be NULL, which doesn't look good for how
 the code reads now. The description for key in virDomainSendSysrq is
 still not sufficient to help me either:
 
 + * @key:SysRq key, like h, c, ...
 
 What does 'h', 'c', ... mean?  What are the options? What do they map to
 functionality wise?  I assume it's hypervisor dependent, but that's all
 stuff you need to describe somewhere. I don't want to guess or go
 searching for the answer through numerous search engine hits.
 
 Looking at the enum Jirka proposed:
 
 typedef enum {
 VIR_DOMAIN_SYSRQ_REBOOT,
 VIR_DOMAIN_SYSRQ_CRASH,
 VIR_DOMAIN_SYSRQ_OOM_KILL,
 VIR_DOMAIN_SYSRQ_SYNC,
 ...
 } virDomainSysrqCommand;
 
 It seems REBOOT would/could be the default. So if key wasn't provided
 the incoming key would be 0 (zero)... If you didn't want a default,
 then you'd have to force a style to be chosen. You're defining the API
 so you show us how you want to handle that. Eventually, each hypervisor
 would map that enum into a character. That is, you'll end up with a way
 to map the enum to a letter for the types of sysrq's each hypervisor
 could support. If a hypervisor doesn't support a specific type of sysrq,
 then decide how to handle.
 
 Anyway given the above enum list, I would think the virsh would be:
 
 virsh sysrq domain reboot
 virsh sysrq domain crash
 virsh sysrq domain kill
 virsh sysrq domain sync
 ...
 
 And key goes from optional to required unless you want to allow 'virsh
 sysrq domain' to mean reboot by default (e.g., if not provided the
 default is to reboot).
 

This still can be implemented using the existing API for sending general
keystrokes to the guest. I still don't see a reason to add a new API as
a special case of an existing one.

Peter



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2] test: fix nwfilter tests following changes in virfirewall.c

2014-12-21 Thread Stefan Berger

On 11/26/2014 11:53 AM, Eric Blake wrote:

On 11/26/2014 09:26 AM, Stefan Berger wrote:

Some of the nwfilter tests are now failing since --concurrent shows
up in the ebtables command. To avoid this, implement a function
preventing the probing for lock support in the eb/iptables tools
and use it in the tests.

Now that I've read Martin and Prerna's exchange, I'm wondering if we
should instead make this override force the locking flags ON, and adjust
the expected test output to expect the -w/--concurrent.



Either this or the other patch should have made it into v1.2.11...

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH V3 1/5] Add public API virDomainSendSysrq

2014-12-21 Thread Chun Yan Liu


 On 12/19/2014 at 08:03 PM, in message 54941429.8000...@redhat.com, John
Ferlan jfer...@redhat.com wrote: 

  
 On 12/19/2014 12:31 AM, Chun Yan Liu wrote: 
  
  
  On 12/18/2014 at 01:00 PM, in message 
  5492d008026600086...@soto.provo.novell.com, Chun Yan Liu 
  cy...@suse.com wrote: 
  
  
  On 12/17/2014 at 06:52 PM, in message 
  20141217105227.gq136...@orkuz.home, 
  Jiri Denemark jdene...@redhat.com wrote: 
  On Wed, Dec 17, 2014 at 16:48:52 +0800, Chunyan Liu wrote: 
  Add public API virDomainSendSysrq for sending SysRequest key. 
  
  Signed-off-by: Chunyan Liu cy...@suse.com 
  --- 
  changes: 
 * add 'flags' to the new API 
 * change parameter from 'const char *key' to 'char key' 
 * change version number from 1.2.11 to 1.2.12 
  
include/libvirt/libvirt-domain.h |  3 +++ 
src/driver-hypervisor.h  |  4  
src/libvirt-domain.c | 39 
  +++ 
src/libvirt_public.syms  |  5 + 
4 files changed, 51 insertions(+) 
  
  diff --git a/include/libvirt/libvirt-domain.h 
  b/include/libvirt/libvirt-domain.h 
  index baef32d..5f72850 100644 
  --- a/include/libvirt/libvirt-domain.h 
  +++ b/include/libvirt/libvirt-domain.h 
  @@ -3526,6 +3526,9 @@ int virDomainGetFSInfo(virDomainPtr dom, 
   virDomainFSInfoPtr **info, 
   unsigned int flags); 
  
  +/* virDomainSendSysrq */ 
  +int virDomainSendSysrq(virDomainPtr dom, char key, unsigned int flags); 
  + 
  
  I think quite a few reviewers (Daniel, Eric, and I) agreed on using an 
  enum instead of char so that the API is more general. 
  
  Sorry, I missed this part. I'll update. One left question: 
  How about 'virsh sysrq' parameters? What would we expect users to pass? 
  
  Any thoughts on that? 
libxl_send_sysrq 
  
 Without a virsh.pod in v3 to go with virsh-domain.c, I'm not sure what  
 you had in mind for syntax previously - although it looks like: 
  
  virsh sysrq domain [key] 

Thanks for reply. The syntax I'm used previously is: 
#virsh sysrq domain key
key is required. It's just a letter, like 'h', 'c', etc. About which options 
can we
have, on can refer to the results on guest through sysrq help. (that is, issue
'virsh sysrq domain h' and look at guest kernel message. I think on each guest,
there must be 'h' option, it will print help message.)

  
 Where if not provided key would be NULL, which doesn't look good for how  
 the code reads now. 

As said above, key is required, it couldn't be NULL, otherwise, it will report 
error.

 The description for key in virDomainSendSysrq is  
 still not sufficient to help me either: 
  
 + * @key:SysRq key, like h, c, ... 
  
 What does 'h', 'c', ... mean?  What are the options? What do they map to  
 functionality wise?  I assume it's hypervisor dependent, but that's all  
 stuff you need to describe somewhere. I don't want to guess or go  
 searching for the answer through numerous search engine hits. 

I can add more description on how one could get those options, but the way
I think is through 'sysrq help' and check guest message.

  
 Looking at the enum Jirka proposed: 
  
 typedef enum { 
  VIR_DOMAIN_SYSRQ_REBOOT, 
  VIR_DOMAIN_SYSRQ_CRASH, 
  VIR_DOMAIN_SYSRQ_OOM_KILL, 
  VIR_DOMAIN_SYSRQ_SYNC, 
  ... 
 } virDomainSysrqCommand; 
  
 It seems REBOOT would/could be the default. So if key wasn't provided  
 the incoming key would be 0 (zero)... If you didn't want a default,  
 then you'd have to force a style to be chosen. You're defining the API  
 so you show us how you want to handle that. Eventually, each hypervisor  
 would map that enum into a character. That is, you'll end up with a way  
 to map the enum to a letter for the types of sysrq's each hypervisor  
 could support. If a hypervisor doesn't support a specific type of sysrq,  
 then decide how to handle. 
  
 Anyway given the above enum list, I would think the virsh would be: 
  
  virsh sysrq domain reboot 
  virsh sysrq domain crash 
  virsh sysrq domain kill 
  virsh sysrq domain sync 
  ... 

OK. That's what I'm concerned and why I hesitated to change API parameter
from 'char key' to 'enum'. Personally I don't think this is a better user
interface and has risk to miss some functionality, since we don't know
which options those hypervisors can support.

I still prefer: 
#virsh sysrq domain key_letter
One can first issue 'virsh sysrq domain h', and check guest kernel message
for all sysrq options. Then send option as he need.
And as a result, I still think I don't see benefit of changing the API parameter
from 'char key' to 'enum'.

How do you think?

Chunyan

 And key goes from optional to required unless you want to allow 'virsh  
 sysrq domain' to mean reboot by default (e.g., if not provided the  
 default is to reboot). 
  
 The string for key would be passed to the virDomainSendSysrq which would  
 then convert or map 

Re: [libvirt] [PATCH V3 1/5] Add public API virDomainSendSysrq

2014-12-21 Thread Chun Yan Liu


 On 12/21/2014 at 10:34 PM, in message 5496da81.40...@redhat.com, Peter 
 Krempa
pkre...@redhat.com wrote: 
 On 12/19/14 13:03, John Ferlan wrote: 
   
   
  On 12/19/2014 12:31 AM, Chun Yan Liu wrote: 
  
  
  On 12/18/2014 at 01:00 PM, in message 
  5492d008026600086...@soto.provo.novell.com, Chun Yan Liu 
  cy...@suse.com wrote: 
  
  
  On 12/17/2014 at 06:52 PM, in message 
  20141217105227.gq136...@orkuz.home, 
  Jiri Denemark jdene...@redhat.com wrote: 
  On Wed, Dec 17, 2014 at 16:48:52 +0800, Chunyan Liu wrote: 
  Add public API virDomainSendSysrq for sending SysRequest key. 
  
  Signed-off-by: Chunyan Liu cy...@suse.com 
  --- 
  changes: 
 * add 'flags' to the new API 
 * change parameter from 'const char *key' to 'char key' 
 * change version number from 1.2.11 to 1.2.12 
  
include/libvirt/libvirt-domain.h |  3 +++ 
src/driver-hypervisor.h  |  4  
src/libvirt-domain.c | 39 
  +++ 
src/libvirt_public.syms  |  5 + 
4 files changed, 51 insertions(+) 
  
  diff --git a/include/libvirt/libvirt-domain.h 
  b/include/libvirt/libvirt-domain.h 
  index baef32d..5f72850 100644 
  --- a/include/libvirt/libvirt-domain.h 
  +++ b/include/libvirt/libvirt-domain.h 
  @@ -3526,6 +3526,9 @@ int virDomainGetFSInfo(virDomainPtr dom, 
   virDomainFSInfoPtr **info, 
   unsigned int flags); 
  
  +/* virDomainSendSysrq */ 
  +int virDomainSendSysrq(virDomainPtr dom, char key, unsigned int 
  flags); 
  + 
  
  I think quite a few reviewers (Daniel, Eric, and I) agreed on using an 
  enum instead of char so that the API is more general. 
  
  Sorry, I missed this part. I'll update. One left question: 
  How about 'virsh sysrq' parameters? What would we expect users to pass? 
  
  Any thoughts on that? 
libxl_send_sysrq 
   
  Without a virsh.pod in v3 to go with virsh-domain.c, I'm not sure what 
  you had in mind for syntax previously - although it looks like: 
   
  virsh sysrq domain [key] 
   
  Where if not provided key would be NULL, which doesn't look good for how 
  the code reads now. The description for key in virDomainSendSysrq is 
  still not sufficient to help me either: 
   
  + * @key:SysRq key, like h, c, ... 
   
  What does 'h', 'c', ... mean?  What are the options? What do they map to 
  functionality wise?  I assume it's hypervisor dependent, but that's all 
  stuff you need to describe somewhere. I don't want to guess or go 
  searching for the answer through numerous search engine hits. 
   
  Looking at the enum Jirka proposed: 
   
  typedef enum { 
  VIR_DOMAIN_SYSRQ_REBOOT, 
  VIR_DOMAIN_SYSRQ_CRASH, 
  VIR_DOMAIN_SYSRQ_OOM_KILL, 
  VIR_DOMAIN_SYSRQ_SYNC, 
  ... 
  } virDomainSysrqCommand; 
   
  It seems REBOOT would/could be the default. So if key wasn't provided 
  the incoming key would be 0 (zero)... If you didn't want a default, 
  then you'd have to force a style to be chosen. You're defining the API 
  so you show us how you want to handle that. Eventually, each hypervisor 
  would map that enum into a character. That is, you'll end up with a way 
  to map the enum to a letter for the types of sysrq's each hypervisor 
  could support. If a hypervisor doesn't support a specific type of sysrq, 
  then decide how to handle. 
   
  Anyway given the above enum list, I would think the virsh would be: 
   
  virsh sysrq domain reboot 
  virsh sysrq domain crash 
  virsh sysrq domain kill 
  virsh sysrq domain sync 
  ... 
   
  And key goes from optional to required unless you want to allow 'virsh 
  sysrq domain' to mean reboot by default (e.g., if not provided the 
  default is to reboot). 
   
  
 This still can be implemented using the existing API for sending general 
 keystrokes to the guest. I still don't see a reason to add a new API as 
 a special case of an existing one. 

First version is implemented by using .domainSendKey but objected. See:
https://www.redhat.com/archives/libvir-list/2014-December/msg00480.html

Thanks.

  
 Peter 
  
  


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH RFC] LXC: don't RO mount /proc, /sys when user namespce enabled

2014-12-21 Thread Chen Hanxiao
If we enabled user ns and provided a uid/gid map,
we do not need to mount /proc, /sys as readonly.
Leave it to kernel for protection.

Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 src/lxc/lxc_container.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 1b9e2f2..3b5845a 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -983,6 +983,12 @@ static int lxcContainerMountBasicFS(bool userns_enabled,
 goto cleanup;
 }
 
+/* don't readonly mount when userns is enabled */
+if (userns_enabled) {
+VIR_FREE(mnt_src);
+continue;
+}
+
 if (bindOverReadonly 
 mount(mnt_src, mnt-dst, NULL,
   MS_BIND|MS_REMOUNT|MS_RDONLY, NULL)  0) {
-- 
1.9.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] lxc: fix show the wrong xml when guest start failed

2014-12-21 Thread Luyao Huang
https://bugzilla.redhat.com/show_bug.cgi?id=1176503

When guest start failed, libvirt will keep the current vm-def,
this will make a issue that we cannot get a right xml after guest
start failed. Pass the newDef to def will make it work well.

Signed-off-by: Luyao Huang lhu...@redhat.com
---
 src/lxc/lxc_process.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 1c0d4e5..b7171ac 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -1353,10 +1353,6 @@ int virLXCProcessStart(virConnectPtr conn,
 VIR_FREE(veths[i]);
 }
 if (rc != 0) {
-if (vm-newDef) {
-virDomainDefFree(vm-newDef);
-vm-newDef = NULL;
-}
 if (priv-monitor) {
 virObjectUnref(priv-monitor);
 priv-monitor = NULL;
@@ -1373,6 +1369,12 @@ int virLXCProcessStart(virConnectPtr conn,
 VIR_FREE(vm-def-seclabels[0]-label);
 VIR_FREE(vm-def-seclabels[0]-imagelabel);
 }
+if (vm-newDef) {
+virDomainDefFree(vm-def);
+vm-def = vm-newDef;
+vm-def-id = -1;
+vm-newDef = NULL;
+}
 }
 for (i = 0; i  nttyFDs; i++)
 VIR_FORCE_CLOSE(ttyFDs[i]);
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list