Re: [libvirt] [PATCH]LXC: free dst before lxcDomainAttachDeviceDiskLive return

2013-09-26 Thread Chen Hanxiao 


 -Original Message-
 From: Daniel Veillard [mailto:veill...@redhat.com]
 Sent: Thursday, September 26, 2013 4:42 PM
 To: Chen Hanxiao
 Cc: libvir-list@redhat.com
 Subject: Re: [libvirt] [PATCH]LXC: free dst before
 lxcDomainAttachDeviceDiskLive return
 
 On Thu, Sep 26, 2013 at 02:01:52PM +0800, Chen Hanxiao wrote:
  From: Chen Hanxiao chenhanx...@cn.fujitsu.com
 
  free dst before lxcDomainAttachDeviceDiskLive return
 
  Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
   virDomainAuditDisk(vm, NULL, def-src, attach, ret == 0);
   if (dst  created  ret  0)
   unlink(dst);
  +VIR_FREE(dst);
   return ret;
   }
 
   Hum, it is a bit complex as the allocated dst is saved in
 the definition a few lines earlier:
 
 /* Labelling normally operates on src, but we need
  * to actally label the dst here, so hack the config */
 def-src = dst;
 
 and then in cleanup it's overriden again with the value saved at the
 start of the function:
 
 cleanup:
 def-src = tmpsrc;
 virDomainAuditDisk(vm, NULL, def-src, attach, ret == 0);
 
 the leak is real and that free certainly need to be added but maybe
 there is a bit of cleanup needed rather than just adding the VIR_FREE()
 

This scenario is a little complex for me. As my investigation, VIR_FREE()
can work
in some cases.

  I will let someone with more knowledge of that code part decide :-)

Thanks for any help :)
 
 Daniel
 
 
 --
 Daniel Veillard  | Open Source and Standards, Red Hat
 veill...@redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
 http://veillard.com/ | virtualization library  http://libvirt.org/


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

[libvirt] [PATCH]LXC: fix a comment typo in lxc_controller.c

2013-09-26 Thread Chen Hanxiao 
From: Chen Hanxiao chenhanx...@cn.fujitsu.com

LXC: fix a comment typo in lxc_controller.c

Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 src/lxc/lxc_controller.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index ed73ab0..636cf6e 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -2011,7 +2011,7 @@ virLXCControllerEventSend(virLXCControllerPtr ctrl,
 virNetMessagePtr msg;
 
 if (!ctrl-client) {
-VIR_WARN(Dropping event %d becuase libvirtd is not connected, 
procnr);
+VIR_WARN(Dropping event %d because libvirtd is not connected, 
procnr);
 return;
 }
 
-- 
1.8.2.1

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

[libvirt] [PATCH]util: Refresh virHook before checking its existence

2013-09-25 Thread Chen Hanxiao 
From: Chen Hanxiao chenhanx...@cn.fujitsu.com

We refresh the status of hook scripts
only when start/stop libvirt, or reload its configuration.
But the status of hooks scripts may be changed.
We need to refresh its status before checking its existence.

Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 src/util/virhook.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/util/virhook.c b/src/util/virhook.c
index 159efdb..5500f62 100644
--- a/src/util/virhook.c
+++ b/src/util/virhook.c
@@ -170,11 +170,15 @@ virHookPresent(int driver) {
 if ((driver  VIR_HOOK_DRIVER_DAEMON) ||
 (driver = VIR_HOOK_DRIVER_LAST))
 return 0;
-if (virHooksFound == -1)
-return 0;
+if (virHookInitialize()  0) {
+if (virHooksFound == -1)
+return 0;
 
-if ((virHooksFound  (1  driver)) == 0)
+if ((virHooksFound  (1  driver)) == 0)
+return 0;
+} else {
 return 0;
+}
 return 1;
 }
 
-- 
1.8.2.1

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

[libvirt] [PATCH]util: Helper function for checking existence of hook files for specific driver

2013-09-25 Thread Chen Hanxiao 
From: Chen Hanxiao chenhanx...@cn.fujitsu.com

We refresh the status of hook scripts
only when start/restart libvirt or reloads its configuration.
But hooks scripts may be changed.
This function will help to check its existence.
And we do not need to start/restart libvirt if
we add/remove hook files.

Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 src/util/virhook.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/util/virhook.c b/src/util/virhook.c
index 159efdb..c4a1a15 100644
--- a/src/util/virhook.c
+++ b/src/util/virhook.c
@@ -129,6 +129,12 @@ virHookCheck(int no, const char *driver) {
 return ret;
 }
 
+static int
+virHookDriverCheck(int driver) {
+return virHookCheck(driver,
+virHookDriverTypeToString(driver));
+}
+
 /*
  * virHookInitialize:
  *
@@ -170,11 +176,12 @@ virHookPresent(int driver) {
 if ((driver  VIR_HOOK_DRIVER_DAEMON) ||
 (driver = VIR_HOOK_DRIVER_LAST))
 return 0;
-if (virHooksFound == -1)
+if (virHookDriverCheck(driver) != 1) {
+VIR_DEBUG(Driver %s hooks files not found,
+  virHookDriverTypeToString(driver));
 return 0;
+}
 
-if ((virHooksFound  (1  driver)) == 0)
-return 0;
 return 1;
 }
 
-- 
1.8.2.1

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

[libvirt] [PATCH]qemu: virDomainControllerFind may return 0 if controller found

2013-09-24 Thread Chen Hanxiao 
From: Chen Hanxiao chenhanx...@cn.fujitsu.com

The return value of virDomainControllerFind =0 means that
the specific controller was found.
But some functions invoke it and treat 0 as not found.
This patch fix these incorrect invocation.

Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 src/qemu/qemu_driver.c  | 2 +-
 src/qemu/qemu_hotplug.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 346a8f9..ccd9cc6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6626,7 +6626,7 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps,
 case VIR_DOMAIN_DEVICE_CONTROLLER:
 controller = dev-data.controller;
 if (virDomainControllerFind(vmdef, controller-type,
-controller-idx)  0) {
+controller-idx) = 0) {
 virReportError(VIR_ERR_OPERATION_INVALID, %s,
_(Target already exists));
 return -1;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 6cdee44..f06930e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -358,7 +358,7 @@ int qemuDomainAttachPciControllerDevice(virQEMUDriverPtr 
driver,
 qemuDomainObjPrivatePtr priv = vm-privateData;
 bool releaseaddr = false;
 
-if (virDomainControllerFind(vm-def, controller-type, controller-idx)  
0) {
+if (virDomainControllerFind(vm-def, controller-type, controller-idx) = 
0) {
 virReportError(VIR_ERR_OPERATION_FAILED,
_(target %s:%d already exists),
type, controller-idx);
-- 
1.8.2.1

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

[libvirt] [Patch]LXC: Add support for attach/detach/update controller in config for LXC

2013-09-24 Thread Chen Hanxiao 
From: Chen Hanxiao chenhanx...@cn.fujitsu.com

Add support for attach/detach/update controller
in config for LXC.

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

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 4cf0b50..9c58f67 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -2750,6 +2750,7 @@ lxcDomainAttachDeviceConfig(virDomainDefPtr vmdef,
 virDomainDiskDefPtr disk;
 virDomainNetDefPtr net;
 virDomainHostdevDefPtr hostdev;
+virDomainControllerDefPtr controller;
 
 switch (dev-type) {
 case VIR_DOMAIN_DEVICE_DISK:
@@ -2787,6 +2788,21 @@ lxcDomainAttachDeviceConfig(virDomainDefPtr vmdef,
 ret = 0;
 break;
 
+case VIR_DOMAIN_DEVICE_CONTROLLER:
+controller = dev-data.controller;
+if (virDomainControllerFind(vmdef, controller-type,
+controller-idx) = 0) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+   _(Target already exists));
+return -1;
+}
+
+if (virDomainControllerInsert(vmdef, controller)  0)
+return -1;
+dev-data.controller = NULL;
+ret = 0;
+break;
+
 default:
  virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
 _(persistent attach of device is not supported));
@@ -2849,6 +2865,7 @@ lxcDomainDetachDeviceConfig(virDomainDefPtr vmdef,
 virDomainDiskDefPtr disk, det_disk;
 virDomainNetDefPtr net;
 virDomainHostdevDefPtr hostdev, det_hostdev;
+virDomainControllerDefPtr controller, det_cont;
 int idx;
 char mac[VIR_MAC_STRING_BUFLEN];
 
@@ -2895,6 +2912,19 @@ lxcDomainDetachDeviceConfig(virDomainDefPtr vmdef,
 break;
 }
 
+case VIR_DOMAIN_DEVICE_CONTROLLER:
+controller = dev-data.controller;
+if ((idx = virDomainControllerFind(vmdef, controller-type,
+   controller-idx))  0) {
+virReportError(VIR_ERR_INVALID_ARG, %s,
+   _(device not present in domain configuration));
+return -1;
+}
+det_cont = virDomainControllerRemove(vmdef, idx);
+virDomainControllerDefFree(det_cont);
+ret = 0;
+break;
+
 default:
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(persistent detach of device is not supported));
-- 
1.8.2.1

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

Re: [libvirt] [Patch]LXC: Add support for attach/detach/update controller in config for LXC

2013-09-24 Thread Chen Hanxiao 
LXC could attach some device to use, such as SCSI disk.

Some device added, libvirt could add controller for it automatically, 
some device new libvirt did not do this.

So we have to deal with this.

 -Original Message-
 From: Daniel P. Berrange [mailto:berra...@redhat.com]
 Sent: Tuesday, September 24, 2013 6:13 PM
 To: Chen Hanxiao
 Cc: libvir-list@redhat.com
 Subject: Re: [libvirt] [Patch]LXC: Add support for attach/detach/update
 controller in config for LXC
 
 On Tue, Sep 24, 2013 at 05:32:28PM +0800, Chen Hanxiao wrote:
  From: Chen Hanxiao chenhanx...@cn.fujitsu.com
 
  Add support for attach/detach/update controller
  in config for LXC.
 
 The LXC driver doesn't use controllers at all, since we just pass
 throw individual disks. What were you trying to achieve with this
 change ?
 
 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]LXC: Check the existence of dir before resolving symlinks

2013-09-23 Thread Chen Hanxiao 
From: Chen Hanxiao chenhanx...@cn.fujitsu.com

If dir does not exist, raise a proper error in logs
and don't let virFileResolveAllLinks throw itself's exceptions
in logs, which may be misleading.

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

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index a979452..c60f5d8 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -1696,6 +1696,7 @@ static int lxcContainerResolveSymlinks(virDomainDefPtr 
vmDef)
 {
 char *newroot;
 size_t i;
+char ebuf[1024];
 
 VIR_DEBUG(Resolving symlinks);
 
@@ -1703,6 +1704,13 @@ static int lxcContainerResolveSymlinks(virDomainDefPtr 
vmDef)
 virDomainFSDefPtr fs = vmDef-fss[i];
 if (!fs-src)
 continue;
+
+if (access(fs-src, F_OK)) {
+VIR_DEBUG(Failed to access '%s': %s, fs-src,
+  virStrerror(errno, ebuf, sizeof(ebuf)));
+return -1;
+}
+
 VIR_DEBUG(Resolving '%s', fs-src);
 if (virFileResolveAllLinks(fs-src, newroot)  0) {
 VIR_DEBUG(Failed to resolve symlink at %s, fs-src);
-- 
1.8.2.1

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

[libvirt] [PATCH]virsh: support readonly in attach-disk command

2013-09-18 Thread Chen Hanxiao 
From: Chen Hanxiao chenhanx...@cn.fujitsu.com

support readonly in attach-disk virsh command
with option --readonly

Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 tools/virsh-domain.c | 7 +++
 tools/virsh.pod  | 5 +++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 3479a1c..d334ebe 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -315,6 +315,10 @@ static const vshCmdOptDef opts_attach_disk[] = {
  .type = VSH_OT_BOOL,
  .help = N_(shareable between domains)
 },
+{.name = readonly,
+ .type = VSH_OT_BOOL,
+ .help = N_(allow guest read-only access to disk)
+},
 {.name = rawio,
  .type = VSH_OT_BOOL,
  .help = N_(needs rawio capability)
@@ -612,6 +616,9 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
 if (vshCommandOptBool(cmd, shareable))
 virBufferAddLit(buf,   shareable/\n);
 
+if (vshCommandOptBool(cmd, readonly))
+virBufferAddLit(buf,   readonly/\n);
+
 if (straddr) {
 if (str2DiskAddress(straddr, diskAddr) != 0) {
 vshError(ctl, _(Invalid address.));
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 0ae5178..91b4429 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1908,8 +1908,8 @@ expected.
 [[[I--live] [I--config] | [I--current]] | [I--persistent]]
 [I--driver driver] [I--subdriver subdriver] [I--cache cache]
 [I--type type] [I--mode mode] [I--config] [I--sourcetype soucetype]
-[I--serial serial] [I--wwn wwn] [I--shareable] [I--rawio]
-[I--address address] [I--multifunction] [I--print-xml]
+[I--serial serial] [I--wwn wwn] [I--shareable] [I--readonly]
+[I--rawio] [I--address address] [I--multifunction] [I--print-xml]
 
 Attach a new disk device to the domain.
 Isource is path for the files and devices. Itarget controls the bus or
@@ -1931,6 +1931,7 @@ Icache can be one of default, none, writethrough, 
writeback,
 directsync or unsafe.
 Iserial is the serial of disk device. Iwwn is the wwn of disk device.
 Ishareable indicates the disk device is shareable between domains.
+Ireadonly indicates the disk device is read-only.
 Irawio indicates the disk needs rawio capability.
 Iaddress is the address of disk device in the form of 
pci:domain.bus.slot.function,
 scsi:controller.bus.unit or ide:controller.bus.unit.
-- 
1.8.2.1

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

[libvirt] [PATCH]LXC: follow the unit style of /proc/meminfo

2013-09-17 Thread Chen Hanxiao 
From: Chen Hanxiao chenhanx...@cn.fujitsu.com

If we start libvirt_lxc with fuse complied, we could see
an isolate /proc/meminfo based on fuse.
Part of this were modified based on container's mem cgroup,
with the unit style of KB;
rest of them were copied from /proc/meminfo, with another
unit style of kB.
It looks a little strange.

We should keep pace with the style of
kernel function meminfo_proc_show,
and change all KB to kB.

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

diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c
index e672b0f..9d12832 100644
--- a/src/lxc/lxc_fuse.c
+++ b/src/lxc/lxc_fuse.c
@@ -165,43 +165,43 @@ static int lxcProcReadMeminfo(char *hostpath, 
virDomainDefPtr def,
 
 if (STREQ(line, MemTotal) 
 (def-mem.hard_limit || def-mem.max_balloon))
-virBufferAsprintf(new_meminfo, MemTotal:   %8llu KB\n,
+virBufferAsprintf(new_meminfo, MemTotal:   %8llu kB\n,
   meminfo.memtotal);
 else if (STREQ(line, MemFree) 
(def-mem.hard_limit || def-mem.max_balloon))
-virBufferAsprintf(new_meminfo, MemFree:%8llu KB\n,
+virBufferAsprintf(new_meminfo, MemFree:%8llu kB\n,
   (meminfo.memtotal - meminfo.memusage));
 else if (STREQ(line, Buffers))
-virBufferAsprintf(new_meminfo, Buffers:%8d KB\n, 0);
+virBufferAsprintf(new_meminfo, Buffers:%8d kB\n, 0);
 else if (STREQ(line, Cached))
-virBufferAsprintf(new_meminfo, Cached: %8llu KB\n,
+virBufferAsprintf(new_meminfo, Cached: %8llu kB\n,
   meminfo.cached);
 else if (STREQ(line, Active))
-virBufferAsprintf(new_meminfo, Active: %8llu KB\n,
+virBufferAsprintf(new_meminfo, Active: %8llu kB\n,
   (meminfo.active_anon + meminfo.active_file));
 else if (STREQ(line, Inactive))
-virBufferAsprintf(new_meminfo, Inactive:   %8llu KB\n,
+virBufferAsprintf(new_meminfo, Inactive:   %8llu kB\n,
   (meminfo.inactive_anon + 
meminfo.inactive_file));
 else if (STREQ(line, Active(anon)))
-virBufferAsprintf(new_meminfo, Active(anon):   %8llu KB\n,
+virBufferAsprintf(new_meminfo, Active(anon):   %8llu kB\n,
   meminfo.active_anon);
 else if (STREQ(line, Inactive(anon)))
-virBufferAsprintf(new_meminfo, Inactive(anon): %8llu KB\n,
+virBufferAsprintf(new_meminfo, Inactive(anon): %8llu kB\n,
   meminfo.inactive_anon);
 else if (STREQ(line, Active(file)))
-virBufferAsprintf(new_meminfo, Active(file):   %8llu KB\n,
+virBufferAsprintf(new_meminfo, Active(file):   %8llu kB\n,
   meminfo.active_file);
 else if (STREQ(line, Inactive(file)))
-virBufferAsprintf(new_meminfo, Inactive(file): %8llu KB\n,
+virBufferAsprintf(new_meminfo, Inactive(file): %8llu kB\n,
   meminfo.inactive_file);
 else if (STREQ(line, Unevictable))
-virBufferAsprintf(new_meminfo, Unevictable:%8llu KB\n,
+virBufferAsprintf(new_meminfo, Unevictable:%8llu kB\n,
   meminfo.unevictable);
 else if (STREQ(line, SwapTotal)  def-mem.swap_hard_limit)
-virBufferAsprintf(new_meminfo, SwapTotal:  %8llu KB\n,
+virBufferAsprintf(new_meminfo, SwapTotal:  %8llu kB\n,
   (meminfo.swaptotal - meminfo.memtotal));
 else if (STREQ(line, SwapFree)  def-mem.swap_hard_limit)
-virBufferAsprintf(new_meminfo, SwapFree:   %8llu KB\n,
+virBufferAsprintf(new_meminfo, SwapFree:   %8llu kB\n,
   (meminfo.swaptotal - meminfo.memtotal -
meminfo.swapusage + meminfo.memusage));
 else {
-- 
1.8.2.1

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

Re: [libvirt] [PATCH v2] Add some notes about security considerations when using LXC

2013-09-13 Thread Chen Hanxiao 

 -Original Message-
 From: Daniel P. Berrange [mailto:berra...@redhat.com]
 Sent: Wednesday, September 11, 2013 6:57 PM
 To: libvir-list@redhat.com
 Cc: Gao feng; Chen Hanxiao; Daniel P. Berrange
 Subject: [PATCH v2] Add some notes about security considerations when
using
 LXC
 
 From: Daniel P. Berrange berra...@redhat.com
 
 Describe some of the issues to be aware of when configuring LXC
 guests with security isolation as a goal.
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  docs/drvlxc.html.in | 103
 
  1 file changed, 103 insertions(+)
 
 In v2:
 
  - Clarify UNIX domain socket issues wrt filesystem  network namespaces
 
 diff --git a/docs/drvlxc.html.in b/docs/drvlxc.html.in
 index 1e6aa1d..66d97e4 100644
 --- a/docs/drvlxc.html.in
 +++ b/docs/drvlxc.html.in
 @@ -168,6 +168,109 @@ Further block or character devices will be made
 available to containers
  depending on their configuration.
  /p
 
 +h2a name=securitySecurity considerations/a/h2
 +
 +p
 +The libvirt LXC driver is fairly flexible in how it can be configured,
 +and as such does not enforce a requirement for strict security
 +separation between a container and the host. This allows it to be used
 +in scenarios where only resource control capabilities are important,
 +and resource sharing is desired. Applications wishing to ensure secure
 +isolation between a container and the host must ensure that they are
 +writing a suitable configuration.
 +/p
 +
 +h3a name=securenetworkingNetwork isolation/a/h3
 +
 +p
 +If the guest configuration does not list any network interfaces,
 +the codenetwork/code namespace will not be activated, and thus
 +the container will see all the host's network interfaces. This will
 +allow apps in the container to bind to/connect from TCP/UDP addresses
 +and ports from the host OS. It also allows applications to access
 +UNIX domain sockets associated with the host OS, which are in the
 +abstract namespace. If access to UNIX domains sockets in the abstract
 +namespace is not wanted, then applications should set the
 +codelt;privnet/gt;/code flag in the
 +codelt;featuresgt;lt;/featuresgt;/code element.
 +/p
 +

This section is very detailed and helpful for developers, but sys admins may
not 
aware of issues like reboot.
Maybe some warnings about 'reboot issue' for sys admins are still needed.

How about keep the v1 patch's description:
Lacking of codenetwork/code namespace would allow coderoot/code
in the container to do anything including shutting down the host OS.

Thanks

 +h3a name=securefsFilesystem isolation/a/h3
 +
 +p
 +If the guest configuration does not list any filesystems, then
 +the container will be set up with a root filesystem that matches
 +the host's root filesystem. As noted earlier, only a few locations
 +such as code/dev/code, code/proc/code and code/sys/code
 +will be altered. This means that, in the absence of restrictions
 +from sVirt, a process running as user/group N:M inside the container
 +will be able to access almost exactly the same files as a process
 +running as user/group N:M in the host.
 +/p
 +
 +p
 +There are multiple options for restricting this. It is possible to
 +simply map the existing root filesystem through to the container in
 +read-only mode. Alternatively a completely separate root filesystem
 +can be configured for the guest. In both cases, further sub-mounts
 +can be applied to customize the content that is made visible. Note
 +that in the absence of sVirt controls, it is still possible for the
 +root user in a container to unmount any sub-mounts applied. The user
 +namespace feature can also be used to restrict access to files based
 +on the UID/GID mappings.
 +/p
 +
 +p
 +Sharing the host filesystem tree, also allows applications to access
 +UNIX domains sockets associated with the host OS, which are in the
 +filesystem namespaces. It should be noted that a number of init
 +systems including at least codesystemd/code and codeupstart/code
 +have UNIX domain socket which are used to control their operation.
 +Thus, if the directory/filesystem holding their UNIX domain socket is
 +exposed to the container, it will be possible for a user in the container
 +to invoke operations on the init service in the same way it could if
 +outside the container. This also applies to other applications in the
 +host which use UNIX domain sockets in the filesystem, such as DBus,
 +Libvirtd, and many more. If this is not desired, then applications
 +should either specify the UID/GID mapping in the configuration to
 +enable user namespaces  thus block access to the UNIX domain socket
 +based on permissions, or should ensure the relevant directories have
 +a bind mount to hide them. This is particularly important for the
 +code/run/code or code/var/run/code directories.
 +/p
 +
 +
 +h3a name=secureusersUser and group isolation/a/h3
 +
 +p
 +If the guest configuration does not list any ID mapping

Re: [libvirt] [PATCH] tools: fix a judgment of equalling zero about an array's length

2013-09-13 Thread Chen Hanxiao 


 -Original Message-
 From: libvir-list-boun...@redhat.com
[mailto:libvir-list-boun...@redhat.com]
 On Behalf Of lawrancejing
 Sent: Friday, September 13, 2013 10:00 AM
 To: libvir-list@redhat.com
 Cc: lawrancejing
 Subject: [libvirt] [PATCH] tools: fix a judgment of equalling zero about
an array's
 length
 
 There is no need to go on executing code when the array's length is zero.
 ---
  tools/virsh-snapshot.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
 index e37a5b3..d7a4c7b 100644
 --- a/tools/virsh-snapshot.c
 +++ b/tools/virsh-snapshot.c
 @@ -239,7 +239,7 @@ vshParseSnapshotMemspec(vshControl *ctl,
 virBufferPtr buf, const char *str)
  return 0;
 
  narray = vshStringToArray(str, array);

If str is NULL, we could get a array sized 1(a NULL element at the end), or
some 
negative values if error occurred.

So don't worry about 0 would return from vshStringToArray. 

 -if (narray  0)
 +if (narray = 0)
  goto cleanup;
 
  for (i = 0; i  narray; i++) {
 --
 1.7.1
 
 --
 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 v2] Add some notes about security considerations when using LXC

2013-09-12 Thread Chen Hanxiao 


 -Original Message-
 From: Daniel P. Berrange [mailto:berra...@redhat.com]
 Sent: Thursday, September 12, 2013 6:46 PM
 To: Chen Hanxiao
 Cc: libvir-list@redhat.com; 'Gao feng'
 Subject: Re: [PATCH v2] Add some notes about security considerations when
 using LXC
 
 On Thu, Sep 12, 2013 at 11:22:18AM +0800, Chen Hanxiao wrote:
 
   -Original Message-
   From: Daniel P. Berrange [mailto:berra...@redhat.com]
   Sent: Wednesday, September 11, 2013 6:57 PM
   To: libvir-list@redhat.com
   Cc: Gao feng; Chen Hanxiao; Daniel P. Berrange
   Subject: [PATCH v2] Add some notes about security considerations when
  using
   LXC
  
   From: Daniel P. Berrange berra...@redhat.com
  
   Describe some of the issues to be aware of when configuring LXC
   guests with security isolation as a goal.
  
   Signed-off-by: Daniel P. Berrange berra...@redhat.com
   ---
docs/drvlxc.html.in | 103
   
1 file changed, 103 insertions(+)
  
   In v2:
  
- Clarify UNIX domain socket issues wrt filesystem  network
 namespaces
  
   diff --git a/docs/drvlxc.html.in b/docs/drvlxc.html.in
   index 1e6aa1d..66d97e4 100644
   --- a/docs/drvlxc.html.in
   +++ b/docs/drvlxc.html.in
   @@ -168,6 +168,109 @@ Further block or character devices will be made
   available to containers
depending on their configuration.
/p
  
   +h2a name=securitySecurity considerations/a/h2
   +
   +p
   +The libvirt LXC driver is fairly flexible in how it can be configured,
   +and as such does not enforce a requirement for strict security
   +separation between a container and the host. This allows it to be used
   +in scenarios where only resource control capabilities are important,
   +and resource sharing is desired. Applications wishing to ensure secure
   +isolation between a container and the host must ensure that they are
   +writing a suitable configuration.
   +/p
   +
   +h3a name=securenetworkingNetwork isolation/a/h3
   +
   +p
   +If the guest configuration does not list any network interfaces,
   +the codenetwork/code namespace will not be activated, and thus
   +the container will see all the host's network interfaces. This will
   +allow apps in the container to bind to/connect from TCP/UDP addresses
   +and ports from the host OS. It also allows applications to access
   +UNIX domain sockets associated with the host OS, which are in the
   +abstract namespace. If access to UNIX domains sockets in the abstract
   +namespace is not wanted, then applications should set the
   +codelt;privnet/gt;/code flag in the
   +codelt;featuresgt;lt;/featuresgt;/code element.
   +/p
   +
 
  This section is very detailed and helpful for developers, but sys admins may
  not
  aware of issues like reboot.
  Maybe some warnings about 'reboot issue' for sys admins are still needed.
 
  How about keep the v1 patch's description:
  Lacking of codenetwork/code namespace would allow
 coderoot/code
  in the container to do anything including shutting down the host OS.
 
 Gao mentioned that UNIX domain sockets in the filesystem namespace
 are not affected by network namespaces. Systemd uses a filesystem
 based socket, so it is filesystem namespaces which are important
 to restrict access to systemd  thus reboot. I already mention
 this later:
 

Thanks for the clarification.
But on RHEL6.4, the upstart do use an abstract socket which is net namespace
aware.
We could prevent it from affecting host OS by enable net namespace.

Maybe we could insert some description about 'upstart' into Network isolation 
section.

Other parts looks great.

Thanks.

   +p
   +Sharing the host filesystem tree, also allows applications to access
   +UNIX domains sockets associated with the host OS, which are in the
   +filesystem namespaces. It should be noted that a number of init
   +systems including at least codesystemd/code and
 codeupstart/code
   +have UNIX domain socket which are used to control their operation.
   +Thus, if the directory/filesystem holding their UNIX domain socket is
   +exposed to the container, it will be possible for a user in the container
   +to invoke operations on the init service in the same way it could if
   +outside the container. This also applies to other applications in the
   +host which use UNIX domain sockets in the filesystem, such as DBus,
   +Libvirtd, and many more. If this is not desired, then applications
   +should either specify the UID/GID mapping in the configuration to
   +enable user namespaces  thus block access to the UNIX domain socket
   +based on permissions, or should ensure the relevant directories have
   +a bind mount to hide them. This is particularly important for the
   +code/run/code or code/var/run/code directories.
   +/p
 
 Daniel
 --
 |: http://berrange.com  -o-
 http://www.flickr.com/photos/dberrange/ :|
 |: http://libvirt.org  -o-
 http://virt-manager.org :|
 |: http://autobuild.org   -o-
 http

[libvirt] [PATCH v3]LXC: Helper function for checking permission of dir when userns enabled

2013-09-10 Thread Chen Hanxiao 
From: Chen Hanxiao chenhanx...@cn.fujitsu.com

If we enable userns, the process with uid/gid in idmap
should have enough permission to access dir we provided
for containers.
Currently, the debug log is very implicit
or misleading sometimes.
This patch will help clarify this for us
when using debug log or virsh.

v2: syntax-check clean

v3: reliable method for checking permission of dir

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

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 8abaea0..9a05e30 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -110,6 +110,13 @@ struct __lxc_child_argv {
 int handshakefd;
 };
 
+typedef struct __lxc_userns_DirPermCheck_argv lxc_userns_DirPermCheck_argv_t;
+struct __lxc_userns_DirPermCheck_argv {
+uid_t uid;
+gid_t gid;
+virDomainDefPtr vmDef;
+};
+
 static int lxcContainerMountFSBlock(virDomainFSDefPtr fs,
 const char *srcprefix);
 
@@ -1829,6 +1836,84 @@ lxcNeedNetworkNamespace(virDomainDefPtr def)
 return false;
 }
 
+static
+int lxcContainerCheckDirPermissionChild(void *argv)
+{
+size_t i;
+lxc_userns_DirPermCheck_argv_t *args = argv;
+uid_t uid = args-uid;
+uid_t gid = args-gid;
+virDomainDefPtr vmDef = args-vmDef;
+char *path;
+
+if (virSetUIDGID(uid, gid, NULL, 0)  0) {
+virReportSystemError(errno, %s,
+  _(setuid or setgid failed));
+_exit(-1);
+}
+
+for (i = 0; i  vmDef-nfss; i++) {
+path = vmDef-fss[i]-src;
+if (access(path, R_OK) || access(path, W_OK) || 
virFileIsExecutable(path)) {
+VIR_DEBUG(Src dir '%s' does not belong to uid/gid: %d/%d,
+  vmDef-fss[i]-src, uid, gid);
+_exit(-1);
+}
+}
+
+_exit(0);
+}
+
+/*
+ * Helper function for helping check
+ * whether we have enough privilege
+ * to operate the source dir when userns enabled
+ * @vmDef: pointer to vm definition structure
+ * Returns 0 on success or -1 in case of error
+ */
+static int
+lxcContainerCheckDirPermission(virDomainDefPtr vmDef)
+{
+uid_t uid;
+gid_t gid;
+int cpid = 0;
+int status;
+char *childStack;
+char *stack;
+int flags =  SIGCHLD;
+
+uid = vmDef-idmap.uidmap[0].target;
+gid = vmDef-idmap.gidmap[0].target;
+
+lxc_userns_DirPermCheck_argv_t args = {
+.uid = uid,
+.gid = gid,
+.vmDef = vmDef
+};
+
+if (VIR_ALLOC_N(stack, getpagesize() * 4)  0)
+return -1;
+
+childStack = stack + (getpagesize() * 4);
+cpid = clone(lxcContainerCheckDirPermissionChild, childStack, flags, 
args);
+VIR_FREE(stack);
+if (cpid  0) {
+virReportSystemError(errno, %s,
+ _(Unable to clone to check permission of 
directory));
+return -1;
+} else if (virProcessWait(cpid, status)  0) {
+return -1;
+}
+
+if (WEXITSTATUS(status) != 0) {
+virReportSystemError(errno, %s,
+ _(Check the permission of source dir provided 
for container));
+return -1;
+}
+
+return 0;
+}
+
 /**
  * lxcContainerStart:
  * @def: pointer to virtual machine structure
@@ -1880,6 +1965,9 @@ int lxcContainerStart(virDomainDefPtr def,
 if (userns_supported()) {
 VIR_DEBUG(Enable user namespace);
 cflags |= CLONE_NEWUSER;
+if (lxcContainerCheckDirPermission(def)  0) {
+return -1;
+}
 } else {
 virReportSystemError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
  _(Kernel doesn't support user namespace));
-- 
1.8.2.1

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

Re: [libvirt] [PATCH]LXC doc: Add warns if net namespace not enabled

2013-09-10 Thread Chen Hanxiao 

 -Original Message-
 From: Daniel P. Berrange [mailto:berra...@redhat.com]
 Sent: Tuesday, September 10, 2013 5:20 PM
 To: Chen Hanxiao
 Cc: libvir-list@redhat.com
 Subject: Re: [libvirt] [PATCH]LXC doc: Add warns if net namespace not enabled
 
 On Mon, Sep 09, 2013 at 04:33:54PM +0800, Chen Hanxiao wrote:
  ping...
+Then this issue could be circumvented./strong /p
   
 h2a name=initDefault container setup/a/h2
 
 Sorry for the delay in responding. While this text looks fine, I think we 
 actually
 need much more content about security issues in LXC. So I'm going to create an
 entire section in the docs about this and include your warning.
 
 I'll copy on you any patch i post.
 

A section about security in docs is needed for users of LXC.
We should give users some instruction or hint when we using LXC.

Looking forward to see your docs.

 
 Daniel
 --



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

Re: [libvirt] [PATCH] Add some notes about security considerations when using LXC

2013-09-10 Thread Chen Hanxiao 

 -Original Message-
 From: Daniel P. Berrange [mailto:berra...@redhat.com]
 Sent: Tuesday, September 10, 2013 6:44 PM
 To: libvir-list@redhat.com
 Cc: Chen Hanxiao; Daniel P. Berrange
 Subject: [PATCH] Add some notes about security considerations when using
LXC
 
 From: Daniel P. Berrange berra...@redhat.com
 
 Describe some of the issues to be aware of when configuring LXC guests
with
 security isolation as a goal.
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  docs/drvlxc.html.in | 93
 +
  1 file changed, 93 insertions(+)
 
 diff --git a/docs/drvlxc.html.in b/docs/drvlxc.html.in index
1e6aa1d..dd2e93c
 100644
 --- a/docs/drvlxc.html.in
 +++ b/docs/drvlxc.html.in
 @@ -168,6 +168,99 @@ Further block or character devices will be made
 available to containers  depending on their configuration.
  /p
 
 +h2a name=securitySecurity considerations/a/h2
 +
 +p
 +The libvirt LXC driver is fairly flexible in how it can be configured,
 +and as such does not enforce a requirement for strict security
 +separation between a container and the host. This allows it to be used
 +in scenarios where only resource control capabilities are important,
 +and resource sharing is desired. Applications wishing to ensure secure
 +isolation between a container and the host must ensure that they are
 +writing a suitable configuration /p
 +
 +h3a name=securenetworkingNetwork isolation/a/h3
 +
 +p
 +If the guest configuration does not list any network interfaces, the
 +codenetwork/code namespace will not be activated, and thus the
 +container will see all the host's network interfaces. This will allow
 +apps in the container to bind to/connect from TCP/UDP addresses and
 +ports from the host OS. It also allows applications to access UNIX
 +domain sockets associated with the host OS.
 +/p
 +
 +p
 +It should be noted that codesystemd/code has a UNIX domain socket
 +hich is used for communication by codesystemctl/code. Thus, with a
 +container that shares the host's network namespace, it will be possible
 +for a user in the container to invoke operations on
 +codesystemd/code in the same way it could if outside the container.
 +In particular this would allow coderoot/code in the container to do
 +anything including shutting down the host OS. If this is not desired,
 +then applications should either specify the UID/GID mapping in the
 +configuration to enable user namespaces, or should set the
 +codelt;privnet/gt;/code flag in the
 codelt;featuresgt;lt;/featuresgt;/code element.
 +/p

There might be too much spotlight on 'systemd'. 
Maybe users may think that this issue only came with systemd.

Actually RHEL6.4GA without systemd still suffer from the reboot issue.
Some apps like upstart can send reboot request to host via unix sockets.

 +
 +
 +h3a name=securefsFilesystem isolation/a/h3
 +
 +p
 +If the guest confuguration does not list any filesystems, then the
 +container will be setup with a root filesystem that matches the host's
 +root filesystem. As noted earlier, only a few locations such as
 +code/dev/code, code/proc/code and code/sys/code will be
 +altered. This means that, in the absence of restrictions from sVirt, a
 +process running as user/group N:M inside the container will be able to
 +access alnmost exactly the same files as a process running as
 +user/group N:M in the host.
 +/p
 +
 +p
 +There are multiple options for restricting this. It is possible to
 +simply map the existing root filesystem through to the container in
 +read-only mode. Alternatively a completely separate root filesystem can
 +be configured for the guest. In both cases, further sub-mounts can be
 +applied to customize the content that is made visible. Note that in the
 +absence of sVirt controls, it is still possible for the root user in a
 +container to unmount any sub-mounts applied. The user namespace feature
 +can also be used to restrict access to files based on the UID/GID
 +mappings.
 +/p
 +
 +h3a name=secureusersUser and group isolation/a/h3
 +
 +p
 +If the guest configuration does not list any ID mapping, then the user
 +and group IDs used inside the container will match those used outside
 +the container. In addition, the capabilities associated with a process
 +in the container will infer the same privileges they would for a
 +process in the host. This has obvious implications for security, since
 +a root user inside the container will be able to access any file owned
 +by root that is visible to the container, and perform more or less any
 +privileged kernel operation. In the absence of additional protection
 +from sVirt, this means that the root user inside a container is
 +effectively as powerful as the root user in the host. There is no
 +security isolation of the root user.
 +/p
 +
 +p
 +The ID mapping facility was introduced to allow for stricter control
 +over the privileges of users inside the container. It allows apps to
 +define rules such as user ID 0

Re: [libvirt] [PATCH]LXC doc: Add warns if net namespace not enabled

2013-09-09 Thread Chen Hanxiao 
ping...

 -Original Message-
 From: libvir-list-boun...@redhat.com
[mailto:libvir-list-boun...@redhat.com]
 On Behalf Of Chen Hanxiao
 Sent: Tuesday, September 03, 2013 10:04 AM
 To: 'Daniel P. Berrange'
 Cc: libvir-list@redhat.com
 Subject: Re: [libvirt] [PATCH]LXC doc: Add warns if net namespace not
enabled
 
 Hi
   Any comments?
 
 Thanks
 
  -Original Message-
  From: Chen Hanxiao [mailto:chenhanx...@cn.fujitsu.com]
  Sent: Friday, August 23, 2013 1:18 PM
  To: libvir-list@redhat.com
  Cc: chenhanx...@cn.fujitsu.com
  Subject: [libvirt][PATCH]LXC doc: Add warns if net namespace not
  enabled
 
  From: Chen Hanxiao chenhanx...@cn.fujitsu.com
 
  If we don't enable network namespace, we could shutdown host by
  executing command 'shutdown' inside container.
  This patch will add some warnings in LXC docs and give some advice to
 readers.
 
  Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
  ---
   docs/drvlxc.html.in |7 +++
   1 files changed, 7 insertions(+), 0 deletions(-)
 
  diff --git a/docs/drvlxc.html.in b/docs/drvlxc.html.in index
  640968f..8f3a36a
  100644
  --- a/docs/drvlxc.html.in
  +++ b/docs/drvlxc.html.in
  @@ -50,6 +50,13 @@ processes inside containers cannot be securely
  isolated from host  process without the use of a mandatory access
  control technology such as SELinux or AppArmor./strong  /p
  +p
  +strongWARNING: If 'net' namespace inot/i enabled for container,
  +host OS could be ishutdown/i by executing command like 'reboot'
  +inside container.br/So make sure 'net' namespace was available and
  +set the lt;privnet/gt; feature in the XML, or configure virtual NICs.
  +Then this issue could be circumvented./strong /p
 
   h2a name=initDefault container setup/a/h2
 
  --
  1.7.1
 
 
 
 
 --
 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

[libvirt] [PATCH] LXC:fix typo in lxc_container.c

2013-09-05 Thread Chen Hanxiao 
From: Chen Hanxiao chenhanx...@cn.fujitsu.com

fix typo in lxc_container.c

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

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 8abaea0..cf096f3 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -1145,7 +1145,7 @@ lxcContainerMountDetectFilesystem(const char *src 
ATTRIBUTE_UNUSED,
 #endif /* ! WITH_BLKID */
 
 /*
- * This functions attempts to do automatic detection of filesystem
+ * This function attempts to do automatic detection of filesystem
  * type following the same rules as the util-linux 'mount' binary.
  *
  * The main difference is that we don't (currently) try to use
@@ -1600,7 +1600,7 @@ static int lxcContainerResolveSymlinks(virDomainDefPtr 
vmDef)
 }
 
 /*
- * This is running as the 'init' process insid the container.
+ * This is running as the 'init' process inside the container.
  * It removes some capabilities that could be dangerous to
  * host system, since they are not currently containerized
  */
@@ -1750,7 +1750,7 @@ static int lxcContainerChild(void *data)
 
 if (lxcContainerSendContinue(argv-handshakefd)  0) {
 virReportSystemError(errno, %s,
-_(failed to send continue signal to controller));
+_(Failed to send continue signal to controller));
 goto cleanup;
 }
 
-- 
1.8.2.1

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

Re: [libvirt] [PATCH]LXC doc: Add warns if net namespace not enabled

2013-09-02 Thread Chen Hanxiao 
Hi
Any comments?

Thanks

 -Original Message-
 From: Chen Hanxiao [mailto:chenhanx...@cn.fujitsu.com]
 Sent: Friday, August 23, 2013 1:18 PM
 To: libvir-list@redhat.com
 Cc: chenhanx...@cn.fujitsu.com
 Subject: [libvirt][PATCH]LXC doc: Add warns if net namespace not enabled
 
 From: Chen Hanxiao chenhanx...@cn.fujitsu.com
 
 If we don't enable network namespace, we could shutdown host by executing
 command 'shutdown' inside container.
 This patch will add some warnings in LXC docs and give some advice to readers.
 
 Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
 ---
  docs/drvlxc.html.in |7 +++
  1 files changed, 7 insertions(+), 0 deletions(-)
 
 diff --git a/docs/drvlxc.html.in b/docs/drvlxc.html.in index 640968f..8f3a36a
 100644
 --- a/docs/drvlxc.html.in
 +++ b/docs/drvlxc.html.in
 @@ -50,6 +50,13 @@ processes inside containers cannot be securely isolated
 from host  process without the use of a mandatory access control technology
 such as SELinux or AppArmor./strong  /p
 +p
 +strongWARNING: If 'net' namespace inot/i enabled for container,
 +host OS could be ishutdown/i by executing command like 'reboot'
 +inside container.br/So make sure 'net' namespace was available and
 +set the lt;privnet/gt; feature in the XML, or configure virtual NICs.
 +Then this issue could be circumvented./strong /p
 
  h2a name=initDefault container setup/a/h2
 
 --
 1.7.1




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

Re: [libvirt] [PATCH]LXC: force container to enable network namespace

2013-08-22 Thread Chen HanXiao 


 -Original Message-
 From: Daniel P. Berrange [mailto:berra...@redhat.com]
 Sent: Thursday, August 22, 2013 5:42 PM
 To: Gao feng
 Cc: Chen Hanxiao; libvir-list@redhat.com
 Subject: Re: [libvirt] [PATCH]LXC: force container to enable network namespace
 
 On Thu, Aug 22, 2013 at 09:23:50AM +0800, Gao feng wrote:
  On 08/21/2013 05:53 PM, Daniel P. Berrange wrote:
   On Wed, Aug 21, 2013 at 05:49:05PM +0800, Chen Hanxiao wrote:
   From: Chen Hanxiao chenhanx...@cn.fujitsu.com
  
   If we don't enable network namespace, we could shutdown host
   inside container by command 'shutdown', which is unacceptable.
   This patch will force users to enable network namespace
   before they start container.
  
   Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
   ---
src/lxc/lxc_container.c |6 ++
1 files changed, 6 insertions(+), 0 deletions(-)
  
   diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
   index c7a22ab..5631ad7 100644
   --- a/src/lxc/lxc_container.c
   +++ b/src/lxc/lxc_container.c
   @@ -1937,6 +1937,12 @@ int lxcContainerStart(virDomainDefPtr def,
if (lxcNeedNetworkNamespace(def)) {
VIR_DEBUG(Enable network namespaces);
cflags |= CLONE_NEWNET;
   +} else {
   +errno = EINVAL;
   +
   +virReportSystemError(errno, %s,
   + _(Network namespace needed));
   +return -1;
}
  
pid = clone(lxcContainerChild, stacktop, cflags, args);
  
   NACK.
  
   We explicitly want to allow containers sharing the host's network
   namespace. If you wish to prevent the problem you describe, then
   set the privnet/ feature in the XML, or configure virtual NICs
  
 
  At least we should give user some warning message or add a notification
  about the probable reboot problem if user doesn't configure private
  net namespace for container.
 
 That can be documented in the drvlxc.html.in page
 

Thanks. New patches will come soon.

 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]LXC doc: Add warns if net namespace not enabled

2013-08-22 Thread Chen Hanxiao 
From: Chen Hanxiao chenhanx...@cn.fujitsu.com

If we don't enable network namespace, we could shutdown host
by executing command 'shutdown' inside container.
This patch will add some warnings in LXC docs and give some
advice to readers.

Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 docs/drvlxc.html.in |7 +++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/docs/drvlxc.html.in b/docs/drvlxc.html.in
index 640968f..8f3a36a 100644
--- a/docs/drvlxc.html.in
+++ b/docs/drvlxc.html.in
@@ -50,6 +50,13 @@ processes inside containers cannot be securely isolated from 
host
 process without the use of a mandatory access control technology
 such as SELinux or AppArmor./strong
 /p
+p
+strongWARNING: If 'net' namespace inot/i enabled for container,
+host OS could be ishutdown/i by executing command like 'reboot'
+inside container.br/So make sure 'net' namespace was available and
+set the lt;privnet/gt; feature in the XML, or configure virtual NICs.
+Then this issue could be circumvented./strong
+/p
 
 h2a name=initDefault container setup/a/h2
 
-- 
1.7.1

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

[libvirt] [PATCH]LXC: force container to enable network namespace

2013-08-21 Thread Chen Hanxiao 
From: Chen Hanxiao chenhanx...@cn.fujitsu.com

If we don't enable network namespace, we could shutdown host
inside container by command 'shutdown', which is unacceptable.
This patch will force users to enable network namespace
before they start container.

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

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index c7a22ab..5631ad7 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -1937,6 +1937,12 @@ int lxcContainerStart(virDomainDefPtr def,
 if (lxcNeedNetworkNamespace(def)) {
 VIR_DEBUG(Enable network namespaces);
 cflags |= CLONE_NEWNET;
+} else {
+errno = EINVAL;
+
+virReportSystemError(errno, %s,
+ _(Network namespace needed));
+return -1;
 }
 
 pid = clone(lxcContainerChild, stacktop, cflags, args);
-- 
1.7.1

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

Re: [libvirt] [PATCH v2]LXC: Helper function for checking ownership of dir when userns enabled

2013-08-20 Thread Chen HanXiao 
Hi
Any comments?

Thanks

 -Original Message-
 From: libvir-list-boun...@redhat.com
[mailto:libvir-list-boun...@redhat.com]
 On Behalf Of Chen HanXiao
 Sent: Wednesday, August 14, 2013 9:30 AM
 To: 'Daniel P. Berrange'
 Cc: libvir-list@redhat.com
 Subject: Re: [libvirt] [PATCH v2]LXC: Helper function for checking
ownership of
 dir when userns enabled
 
 
 
  -Original Message-
  From: Daniel P. Berrange [mailto:berra...@redhat.com]
  Sent: Saturday, August 10, 2013 12:54 AM
  To: Chen Hanxiao
  Cc: libvir-list@redhat.com
  Subject: Re: [libvirt] [PATCH v2]LXC: Helper function for checking
ownership of
  dir when userns enabled
 
  On Fri, Aug 09, 2013 at 04:05:58PM +0800, Chen Hanxiao wrote:
   From: Chen Hanxiao chenhanx...@cn.fujitsu.com
  
   If we enable userns, the ownership of dir we provided for containers
   should match the uid/gid in idmap.
   Currently, the debug log is very implicit or misleading sometimes.
   This patch will help clarify this for us when using
   debug log or virsh.
 
  I do recall hitting some permission issue once, but can't remember
  just what it was. Can you describe exactly how to reproduce the
  problem ?
 
 
 1)  Enable user namespace in kernel
 2)  Add idmap for container
 3)  Don't change the ownership of devices/ filesystem/ source dir  ( leave
 them to 'root' for instance)
 4)  Start the container
 
 Usually I got an input/output error by virsh, which is not a good hint.
 
 
   Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
   ---
src/lxc/lxc_container.c |   46
  ++
1 files changed, 46 insertions(+), 0 deletions(-)
  
   diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
   index b910b10..2ccdc61 100644
   --- a/src/lxc/lxc_container.c
   +++ b/src/lxc/lxc_container.c
   @@ -1815,6 +1815,49 @@ lxcNeedNetworkNamespace(virDomainDefPtr
  def)
return false;
}
  
   +/*
   + * Helper function for helping check
   + * whether we have enough privilege
   + * to operate the source dir when userns enabled
   + * @vmDef: pointer to vm definition structure
   + * Returns 0 on success or -1 in case of error
   + */
   +static int
   +lxcContainerUsernsSrcOwnershipCheck(virDomainDefPtr vmDef)
   +{
   +struct stat buf;
   +size_t i;
   +uid_t uid;
   +gid_t gid;
   +
   +VIR_DEBUG(vmDef-nfss %d, (int)vmDef-nfss);
   +for (i = 0; i  vmDef-nfss; i++) {
   +VIR_DEBUG(dst is %s, src is %s,
   +  vmDef-fss[i]-dst,
   +  vmDef-fss[i]-src);
   +
   +uid = vmDef-idmap.uidmap[0].target;
   +gid = vmDef-idmap.gidmap[0].target;
   +
   +if (lstat(vmDef-fss[i]-src, buf)  0) {
   +virReportSystemError(errno, _(Cannot access '%s'),
   + vmDef-fss[i]-src);
   +return -1;
   +} else if (uid != buf.st_uid || gid != buf.st_gid) {
   +VIR_DEBUG(In userns uid is %d, gid is %d\n,
   +  uid, gid);
   +errno = EINVAL;
   +
   +virReportSystemError(errno,
   +  _([userns] Src dir '%s' does not
  belong to uid/gid: %d/%d),
   + vmDef-fss[i]-src, uid, gid);
   +return -1;
   +}
   +}
   +
   +return 0;
   +}
   +
/**
 * lxcContainerStart:
 * @def: pointer to virtual machine structure
   @@ -1866,6 +1909,9 @@ int lxcContainerStart(virDomainDefPtr def,
if (userns_supported()) {
VIR_DEBUG(Enable user namespace);
cflags |= CLONE_NEWUSER;
   +if (lxcContainerUsernsSrcOwnershipCheck(def)  0) {
   +return -1;
   +}
} else {
virReportSystemError(VIR_ERR_CONFIG_UNSUPPORTED,
  %s,
 _(Kernel doesn't support user
  namespace));
 
 
  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


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

Re: [libvirt] [PATCH v2]LXC: Helper function for checking ownership of dir when userns enabled

2013-08-13 Thread Chen HanXiao 


 -Original Message-
 From: Daniel P. Berrange [mailto:berra...@redhat.com]
 Sent: Saturday, August 10, 2013 12:54 AM
 To: Chen Hanxiao
 Cc: libvir-list@redhat.com
 Subject: Re: [libvirt] [PATCH v2]LXC: Helper function for checking ownership 
 of
 dir when userns enabled
 
 On Fri, Aug 09, 2013 at 04:05:58PM +0800, Chen Hanxiao wrote:
  From: Chen Hanxiao chenhanx...@cn.fujitsu.com
 
  If we enable userns, the ownership of dir we provided for containers
  should match the uid/gid in idmap.
  Currently, the debug log is very implicit or misleading sometimes.
  This patch will help clarify this for us when using
  debug log or virsh.
 
 I do recall hitting some permission issue once, but can't remember
 just what it was. Can you describe exactly how to reproduce the
 problem ?
 

1)  Enable user namespace in kernel
2)  Add idmap for container
3)  Don't change the ownership of devices/ filesystem/ source dir  ( leave them 
to 'root' for instance)
4)  Start the container

Usually I got an input/output error by virsh, which is not a good hint.


  Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
  ---
   src/lxc/lxc_container.c |   46
 ++
   1 files changed, 46 insertions(+), 0 deletions(-)
 
  diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
  index b910b10..2ccdc61 100644
  --- a/src/lxc/lxc_container.c
  +++ b/src/lxc/lxc_container.c
  @@ -1815,6 +1815,49 @@ lxcNeedNetworkNamespace(virDomainDefPtr
 def)
   return false;
   }
 
  +/*
  + * Helper function for helping check
  + * whether we have enough privilege
  + * to operate the source dir when userns enabled
  + * @vmDef: pointer to vm definition structure
  + * Returns 0 on success or -1 in case of error
  + */
  +static int
  +lxcContainerUsernsSrcOwnershipCheck(virDomainDefPtr vmDef)
  +{
  +struct stat buf;
  +size_t i;
  +uid_t uid;
  +gid_t gid;
  +
  +VIR_DEBUG(vmDef-nfss %d, (int)vmDef-nfss);
  +for (i = 0; i  vmDef-nfss; i++) {
  +VIR_DEBUG(dst is %s, src is %s,
  +  vmDef-fss[i]-dst,
  +  vmDef-fss[i]-src);
  +
  +uid = vmDef-idmap.uidmap[0].target;
  +gid = vmDef-idmap.gidmap[0].target;
  +
  +if (lstat(vmDef-fss[i]-src, buf)  0) {
  +virReportSystemError(errno, _(Cannot access '%s'),
  + vmDef-fss[i]-src);
  +return -1;
  +} else if (uid != buf.st_uid || gid != buf.st_gid) {
  +VIR_DEBUG(In userns uid is %d, gid is %d\n,
  +  uid, gid);
  +errno = EINVAL;
  +
  +virReportSystemError(errno,
  +  _([userns] Src dir '%s' does not
 belong to uid/gid: %d/%d),
  + vmDef-fss[i]-src, uid, gid);
  +return -1;
  +}
  +}
  +
  +return 0;
  +}
  +
   /**
* lxcContainerStart:
* @def: pointer to virtual machine structure
  @@ -1866,6 +1909,9 @@ int lxcContainerStart(virDomainDefPtr def,
   if (userns_supported()) {
   VIR_DEBUG(Enable user namespace);
   cflags |= CLONE_NEWUSER;
  +if (lxcContainerUsernsSrcOwnershipCheck(def)  0) {
  +return -1;
  +}
   } else {
   virReportSystemError(VIR_ERR_CONFIG_UNSUPPORTED,
 %s,
_(Kernel doesn't support user
 namespace));
 
 
 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 v2]LXC: Helper function for checking ownership of dir when userns enabled

2013-08-09 Thread Chen Hanxiao 
From: Chen Hanxiao chenhanx...@cn.fujitsu.com

If we enable userns, the ownership of dir we provided for containers
should match the uid/gid in idmap.
Currently, the debug log is very implicit or misleading sometimes.
This patch will help clarify this for us when using
debug log or virsh.

v2: syntax-check clean

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

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index b910b10..2ccdc61 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -1815,6 +1815,49 @@ lxcNeedNetworkNamespace(virDomainDefPtr def)
 return false;
 }
 
+/*
+ * Helper function for helping check
+ * whether we have enough privilege
+ * to operate the source dir when userns enabled
+ * @vmDef: pointer to vm definition structure
+ * Returns 0 on success or -1 in case of error
+ */
+static int
+lxcContainerUsernsSrcOwnershipCheck(virDomainDefPtr vmDef)
+{
+struct stat buf;
+size_t i;
+uid_t uid;
+gid_t gid;
+
+VIR_DEBUG(vmDef-nfss %d, (int)vmDef-nfss);
+for (i = 0; i  vmDef-nfss; i++) {
+VIR_DEBUG(dst is %s, src is %s,
+  vmDef-fss[i]-dst,
+  vmDef-fss[i]-src);
+
+uid = vmDef-idmap.uidmap[0].target;
+gid = vmDef-idmap.gidmap[0].target;
+
+if (lstat(vmDef-fss[i]-src, buf)  0) {
+virReportSystemError(errno, _(Cannot access '%s'),
+ vmDef-fss[i]-src);
+return -1;
+} else if (uid != buf.st_uid || gid != buf.st_gid) {
+VIR_DEBUG(In userns uid is %d, gid is %d\n,
+  uid, gid);
+errno = EINVAL;
+
+virReportSystemError(errno,
+  _([userns] Src dir '%s' does not belong to 
uid/gid: %d/%d),
+ vmDef-fss[i]-src, uid, gid);
+return -1;
+}
+}
+
+return 0;
+}
+
 /**
  * lxcContainerStart:
  * @def: pointer to virtual machine structure
@@ -1866,6 +1909,9 @@ int lxcContainerStart(virDomainDefPtr def,
 if (userns_supported()) {
 VIR_DEBUG(Enable user namespace);
 cflags |= CLONE_NEWUSER;
+if (lxcContainerUsernsSrcOwnershipCheck(def)  0) {
+return -1;
+}
 } else {
 virReportSystemError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
  _(Kernel doesn't support user namespace));
-- 
1.7.1

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

[libvirt] LXC: Helper function for checking ownership of dir when userns enabled

2013-08-08 Thread Chen Hanxiao 
From: Chen Hanxiao chenhanx...@cn.fujitsu.com

 If we enable userns, the ownership of dir we provided for containers
 should match the uid/gid in idmap.
 Currently, the debug log is very implicit or misleading sometimes.
 This patch will help clarify this for us when using
 debug log or virsh.

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

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index b910b10..ce17466 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -1815,6 +1815,48 @@ lxcNeedNetworkNamespace(virDomainDefPtr def)
 return false;
 }
 
+/*
+ * Helper function for helping check
+ * whether we have enough privilege 
+ * to operate the source dir when userns enabled  
+ * @vmDef: pointer to vm definition structure
+ * Returns 0 on success or -1 in case of error
+ */
+static int
+lxcContainerUsernsSrcOwnershipCheck(virDomainDefPtr vmDef)
+{
+struct stat buf;
+int i;
+uid_t uid;
+gid_t gid;
+
+for(i=0; i  vmDef-nfss; i++) {
+VIR_DEBUG(dst is %s, src is %s,
+vmDef-fss[i]-dst,
+vmDef-fss[i]-src);
+
+uid = vmDef-idmap.uidmap[0].target;
+gid = vmDef-idmap.gidmap[0].target;
+
+if (lstat(vmDef-fss[i]-src, buf)  0) {
+virReportSystemError(errno, _(Cannot access '%s'),
+ vmDef-fss[i]-src);
+return -1;
+} else if(uid != buf.st_uid || gid != buf.st_gid) {
+VIR_DEBUG(In userns uid is %d, gid is %d\n,
+uid, gid);
+errno = EINVAL;
+
+virReportSystemError(errno, 
+[userns] Src dir \%s\ does not belong to uid/gid:%d/%d,
+vmDef-fss[i]-src, uid, gid);
+return -1;
+}
+}
+
+return 0;
+}
+
 /**
  * lxcContainerStart:
  * @def: pointer to virtual machine structure
@@ -1866,6 +1908,9 @@ int lxcContainerStart(virDomainDefPtr def,
 if (userns_supported()) {
 VIR_DEBUG(Enable user namespace);
 cflags |= CLONE_NEWUSER;
+if(lxcContainerUsernsSrcOwnershipCheck(def)  0) {
+return -1;
+}
 } else {
 virReportSystemError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
  _(Kernel doesn't support user namespace));
-- 
1.7.1

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

[libvirt] [PATCH]Support virtio-scsi disk XML with tag 'model'

2012-11-12 Thread Chen Hanxiao 
From: ChenHanxiao chenhanx...@cn.fujitsu.com

If we add a virtio-disk, we need to add a SCSI controller with
model 'virtio-scsi'.
This patch allows libvirt to analyze disks XML with tag 'model':

disk type='block' device='disk'
driver name='qemu' type='raw'/
source dev='/dev/sdb'/
target dev='sda' bus='scsi' model = 'virtio-scsi'/
/disk

If we got a disks XML with bus='scsi' model = 'virtio-scsi',
we'll add a SCSI controller with model 'virtio-scsi' automatically.

Signed-off-by: ChenHanxiao chenhanx...@cn.fujitsu.com
---
 src/conf/domain_conf.c |   29 -
 src/conf/domain_conf.h |2 ++
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 99f03a9..0032df4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3528,6 +3528,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
 virDomainDiskHostDefPtr hosts = NULL;
 int nhosts = 0;
 char *bus = NULL;
+char *model = NULL;
 char *cachetag = NULL;
 char *error_policy = NULL;
 char *rerror_policy = NULL;
@@ -3667,6 +3668,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
 target = virXMLPropString(cur, dev);
 bus = virXMLPropString(cur, bus);
 tray = virXMLPropString(cur, tray);
+model = virXMLPropString(cur, model);
 
 /* HACK: Work around for compat with Xen
  * driver in previous libvirt releases */
@@ -4015,6 +4017,14 @@ virDomainDiskDefParseXML(virCapsPtr caps,
 }
 }
 
+if (model) {
+if ((def-model = virDomainControllerModelSCSITypeFromString(model))  
0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+_(unknown disk bus model type '%s'), bus);
+goto error;
+}
+}
+
 if (tray) {
 if ((def-tray_status = virDomainDiskTrayTypeFromString(tray))  0) {
 virReportError(VIR_ERR_XML_ERROR,
@@ -4221,6 +4231,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
 
 cleanup:
 VIR_FREE(bus);
+VIR_FREE(model);
 VIR_FREE(type);
 VIR_FREE(snapshot);
 VIR_FREE(rawio);
@@ -8499,6 +8510,7 @@ virDomainLookupVcpuPin(virDomainDefPtr def,
 
 static int virDomainDefMaybeAddController(virDomainDefPtr def,
   int type,
+  int model,
   int idx)
 {
 int found = 0;
@@ -8521,7 +8533,7 @@ static int virDomainDefMaybeAddController(virDomainDefPtr 
def,
 
 cont-type = type;
 cont-idx = idx;
-cont-model = -1;
+cont-model = model;
 
 if (cont-type == VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL) {
 cont-opts.vioserial.ports = -1;
@@ -9508,7 +9520,8 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr 
caps,
 if (def-virtType == VIR_DOMAIN_VIRT_QEMU ||
 def-virtType == VIR_DOMAIN_VIRT_KQEMU ||
 def-virtType == VIR_DOMAIN_VIRT_KVM)
-if (virDomainDefMaybeAddController(def, 
VIR_DOMAIN_CONTROLLER_TYPE_USB, 0)  0)
+if (virDomainDefMaybeAddController(def, 
VIR_DOMAIN_CONTROLLER_TYPE_USB, 
+VIR_DOMAIN_CONTROLLER_MODEL_UNDEF, 0)  0)
 goto error;
 
 /* analysis of the resource leases */
@@ -11390,6 +11403,7 @@ static int 
virDomainDefAddDiskControllersForType(virDomainDefPtr def,
 {
 int i;
 int maxController = -1;
+int model = VIR_DOMAIN_CONTROLLER_MODEL_UNDEF;
 
 for (i = 0 ; i  def-ndisks ; i++) {
 if (def-disks[i]-bus != diskBus)
@@ -11400,10 +11414,12 @@ static int 
virDomainDefAddDiskControllersForType(virDomainDefPtr def,
 
 if ((int)def-disks[i]-info.addr.drive.controller  maxController)
 maxController = def-disks[i]-info.addr.drive.controller;
+
+model = def-disks[i]-model;
 }
 
 for (i = 0 ; i = maxController ; i++) {
-if (virDomainDefMaybeAddController(def, controllerType, i)  0)
+if (virDomainDefMaybeAddController(def, controllerType, model, i)  0)
 return -1;
 }
 
@@ -11425,7 +11441,8 @@ static int 
virDomainDefMaybeAddVirtioSerialController(virDomainDefPtr def)
 idx = channel-info.addr.vioserial.controller;
 
 if (virDomainDefMaybeAddController(def,
-VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL, idx)  0)
+VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL, 
+VIR_DOMAIN_CONTROLLER_MODEL_UNDEF, idx)  0)
 return -1;
 }
 }
@@ -11440,7 +11457,8 @@ static int 
virDomainDefMaybeAddVirtioSerialController(virDomainDefPtr def)
 idx = console-info.addr.vioserial.controller;
 
 if (virDomainDefMaybeAddController(def,
-VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL, idx)  0)
+VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL, 
+VIR_DOMAIN_CONTROLLER_MODEL_UNDEF, idx)  0)

Re: [libvirt] [PATCH]Support virtio-scsi disk XML with tag 'model'

2012-11-12 Thread Chen HanXiao 
 -Original Message-
 From: Daniel P. Berrange [mailto:berra...@redhat.com]
 Sent: Monday, November 12, 2012 6:31 PM
 To: Chen Hanxiao
 Cc: libvir-list@redhat.com
 Subject: Re: [libvirt] [PATCH]Support virtio-scsi disk XML with tag 'model'
 
 On Mon, Nov 12, 2012 at 05:18:41PM +0800, Chen Hanxiao wrote:
  From: ChenHanxiao chenhanx...@cn.fujitsu.com
 
  If we add a virtio-disk, we need to add a SCSI controller with
  model 'virtio-scsi'.
  This patch allows libvirt to analyze disks XML with tag 'model':
 
  disk type='block' device='disk'
  driver name='qemu' type='raw'/
  source dev='/dev/sdb'/
  target dev='sda' bus='scsi' model = 'virtio-scsi'/
  /disk
 
  If we got a disks XML with bus='scsi' model = 'virtio-scsi',
  we'll add a SCSI controller with model 'virtio-scsi' automatically.
 
  Signed-off-by: ChenHanxiao chenhanx...@cn.fujitsu.com
 
 NACK duplicating the bus information on the disk is just
 not very pleasant. Applications should really be adding
 controllers explicitly - the auto-add of controllers is
 just to provide back-compat for applications that were
 written to add IDE disks before the controller concept
 was introduced. New apps should not rely on this.

Adding SATA, SCSI, IDE disk for instance,
Did this means that functions in libvirt such as:
virDomainDefAddImplicitControllers
virDomainDefAddDiskControllersForType
is not helper function for adding controller of specific devices
but to make the old code compatible with newly introduced
controller concept?  

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] [RFC][PATCH] Revision for message payload encoding error when adding a large mount of virtio disks

2012-04-19 Thread Chen Hanxiao 
From: Chen Hanxiao chenhanx...@cn.fujitsu.com

When adding a large number of virtio storage devices to
virtual machine by using 'virsh edit' command, there is
a problem:
When we added more than 190 virtio disks by 'virsh edit'
command, we got a feedback as 'error: Unable to encode
message payload'. In virt-mananger, the same defect occurs
with about 180 virtio disks added.

PCI devices are limited by the virtualized system
architecture. Out of the 32 available PCI devices for a
guest, 4 are not removable. This means there are up to
28 free PCI slots available for additional devices per
guest. Each PCI device in a guest can have up to 8 functions.
One slot kept for network interface, we could add at most
216 (27*8) virtio disks.
The length of xml description with multifuction for one virtio
disk is about 290 characters; In virt-manger, an extra tag
'alias name' will come to the xml description, which would add
about 40 more characters.
So for one xml description, 330 characters would be enough.
A brand new virtual machine with one IDE bus disk needs about
1900 characters for xml description.

In src/remote/remote_protocol.x, there is a limitation for XDR
enoding length as REMOTE_STRING_MAX.
According to the analysis above, at least 73180(1900+216*330)
characters are needed for 216 virtio disks. In the view of
variable length in tag 'source file', so I think it would be
better to extend this limitation from 65536 to 8.
---
 src/remote/remote_protocol.x |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 2d57247..2c8dcbb 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -65,7 +65,7 @@
  * This is an arbitrary limit designed to stop the decoder from trying
  * to allocate unbounded amounts of memory when fed with a bad message.
  */
-const REMOTE_STRING_MAX = 65536;
+const REMOTE_STRING_MAX = 8;
 
 /* A long string, which may NOT be NULL. */
 typedef string remote_nonnull_stringREMOTE_STRING_MAX;
-- 
1.7.1

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

Re: [libvirt] [RFC][PATCH] Revision for message payload encoding error when adding a large mount of virtio disks

2012-04-19 Thread Chen HanXiao 
On Thu, Apr 19, 2012 at 03:04:09PM +0200, Michal Privoznik wrote:
 On 19.04.2012 14:45, Richard W.M. Jones wrote:
  On Thu, Apr 19, 2012 at 02:25:16PM +0800, Chen Hanxiao wrote:
* This is an arbitrary limit designed to stop the decoder from trying
* to allocate unbounded amounts of memory when fed with a bad message.
*/
  -const REMOTE_STRING_MAX = 65536;
  +const REMOTE_STRING_MAX = 8;
  
  Can this limit be changed?  I thought it would break existing clients
  or servers.
  
  Rich.
  
 
If we're going to change it, I say change it to something much bigger.

The original limit was very conservative, and was based on some
assumptions about wanting to keep buffers on the stack which is no
longer true in modern libvirt code.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/


It would be beter to change current REMOTE_STRING_MAX to a bigger number.
But VIR_NET_MESSAGE_MAX limited the buffer for XDR stream as 
4*REMOTE_STRING_MAX in
current libvirt.
So could the REMOTE_STRING_MAX change as half as the VIR_NET_MESSAGE_MAX?

-const REMOTE_STRING_MAX = 65536;
+const REMOTE_STRING_MAX = 131072;  //twice as many as before


It's accuracy to allocate the buffer size 4 times as much as message size.
According to the real message, most of them are strings. So I think 
REMOTE_STRING_MAX with half size of VIR_NET_MESSAGE_MAX is not 
a bad choice.
I test this change on my machine, it seems to work fine.

If Michal's dynamically allocating buffer function finished, we could change 
this 
limit to a bigger one.

--
Regards

Chen HanXiao

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

<    1   2   3   4   5   6