Re: [libvirt] [PATCH 3/3] storage: Split out volume upload/download as separate backend function

2014-07-11 Thread Peter Krempa
On 07/10/14 18:48, Eric Blake wrote:
 On 07/10/2014 08:30 AM, Peter Krempa wrote:
 For non-local storage drivers we can't expect to use the FDStream
 backend for up/downloading volumes. Split the code into a separate
 backend function so that we can add protocol specific code later.
 ---
  src/storage/storage_backend.c | 31 +++
  src/storage/storage_backend.h | 30 ++
  src/storage/storage_backend_disk.c|  2 ++
  src/storage/storage_backend_fs.c  |  7 ++
  src/storage/storage_backend_iscsi.c   |  2 ++
  src/storage/storage_backend_logical.c |  2 ++
  src/storage/storage_backend_mpath.c   |  2 ++
  src/storage/storage_backend_scsi.c|  2 ++
  src/storage/storage_driver.c  | 47 
 +++
  9 files changed, 93 insertions(+), 32 deletions(-)
 
 Without even reading this patch yet, this is more the approach I had in
 mind for 2/3.
 

I've pushed this one according to Jan's ACK and will re-do 2/3 in the
same fashion.

Peter



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

[libvirt] [PATCHv2 1/2] doc: Document that snapshot name of block-backed disk isnt autogenerated

2014-07-11 Thread Peter Krempa
Libvirt generates external snapshot target file names for file backed
storage but not for block backed storage. Document the limitation.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1032363
---
 docs/formatsnapshot.html.in | 8 +---
 tools/virsh.pod | 4 +++-
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in
index 1f9a6c6..4f7b7b2 100644
--- a/docs/formatsnapshot.html.in
+++ b/docs/formatsnapshot.html.in
@@ -164,9 +164,11 @@
 attribute codetype/code giving the driver type (such
 as qcow2), of the new file created by the external
 snapshot of the new file.  If codesource/code is not
-given, a file name is generated that consists of the
-existing file name with anything after the trailing dot
-replaced by the snapshot name.  Remember that with external
+given and the disk is backed by a local image file (not
+a block device or remote storage), a file name is
+generated that consists of the existing file name
+with anything after the trailing dot replaced by the
+snapshot name.  Remember that with external
 snapshots, the original file name becomes the read-only
 snapshot, and the new file name contains the read-write
 delta of all disk changes since the snapshot.
diff --git a/tools/virsh.pod b/tools/virsh.pod
index a5e8406..5da71c3 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -3219,7 +3219,9 @@ The I--diskspec option can be used to control how 
I--disk-only and
 external checkpoints create external files.  This option can occur
 multiple times, according to the number of disk elements in the domain
 xml.  Each diskspec is in the
-form Bdisk[,snapshot=type][,driver=type][,file=name].  To include a
+form Bdisk[,snapshot=type][,driver=type][,file=name].  A Idiskspec
+must be provided for disks backed by block devices as libvirt doesn't
+auto-generate file names for those.  To include a
 literal comma in Bdisk or in Bfile=name, escape it with a second
 comma.  A literal I--diskspec must precede each Bdiskspec unless
 all three of Idomain, Iname, and Idescription are also present.
-- 
2.0.0

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


[libvirt] [PATCHv2 2/2] doc: Be more specific about semantics of _REUSE_EXT flag

2014-07-11 Thread Peter Krempa
Snapshots and block-copy have a flag that forces qemu to re-use existing
file. Our docs weren't exactly clear on what the existing file should
contain for this to actually work.

Re-word the docs a bit to state that the file needs to be pre-created in
the desired format and the backing chain metadata needs to be set prior
to handing it over to qemu.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1084360
---
 src/libvirt.c   | 23 ++-
 tools/virsh.pod | 17 ++---
 2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c
index 8684298..79bcdf1 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -18394,10 +18394,12 @@ virDomainSnapshotGetConnect(virDomainSnapshotPtr 
snapshot)
  * destination files already exist as a non-empty regular file, the
  * snapshot is rejected to avoid losing contents of those files.
  * However, if @flags includes VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT,
- * then the destination files must already exist and contain content
- * identical to the source files (this allows a management app to
- * pre-create files with relative backing file names, rather than the
- * default of creating with absolute backing file names).
+ * then the destination files must be pre-created manually with
+ * the correct image format and metadata including backing store path
+ * (this allows a management app to pre-create files with relative backing
+ * file names, rather than the default of creating with absolute backing
+ * file names). Note that setting incorrect metadata in the pre-created
+ * image may lead to the VM being unable to start.
  *
  * Be aware that although libvirt prefers to report errors up front with
  * no other effect, some hypervisors have certain types of failures where
@@ -19864,11 +19866,14 @@ virDomainBlockPull(virDomainPtr dom, const char *disk,
  * by adding VIR_DOMAIN_BLOCK_REBASE_COPY_RAW to force the copy to be raw
  * (does not make sense with the shallow flag unless the source is also raw),
  * or by using VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT to reuse an existing file
- * with initial contents identical to the backing file of the source (this
- * allows a management app to pre-create files with relative backing file
- * names, rather than the default of absolute backing file names; as a
- * security precaution, you should generally only use reuse_ext with the
- * shallow flag and a non-raw destination file).
+ * which was pre-created with the correct format and metadata and sufficient
+ * size to hold the copy. In case the VIR_DOMAIN_BLOCK_REBASE_SHALLOW flag
+ * is used the pre-created file has to exhibit the same guest visible contents
+ * as the backing file of the original image. This allows a management app to
+ * pre-create files with relative backing file names, rather than the default
+ * of absolute backing file names; as a security precaution, you should
+ * generally only use reuse_ext with the shallow flag and a non-raw
+ * destination file.
  *
  * A copy job has two parts; in the first phase, the @bandwidth parameter
  * affects how fast the source is pulled into the destination, and the job
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 5da71c3..1a2b01f 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -876,10 +876,11 @@ flattens the entire chain; but if I--shallow is 
specified, the copy
 shares the backing chain.

 If I--reuse-external is specified, then Idest must exist and have
-contents identical to the resulting backing file (that is, it must
-start with contents matching the backing file Idisk if I--shallow
-is used, otherwise it must start empty); this option is typically used
-to set up a relative backing file name in the destination.
+sufficient space to hold the copy. If I--shallow is used in
+conjunction with I--reuse-external then the pre-created image must have
+guest visible contents identical to guest visible contents of the backing
+file of the original image. This may be used to modify the backing file
+names on the destination.

 The format of the destination is determined by the first match in the
 following list: if I--raw is specified, it will be raw; if
@@ -3172,7 +3173,8 @@ metadata again).

 If I--reuse-external is specified, and the snapshot XML requests an
 external snapshot with a destination of an existing file, then the
-destination must exist, and is reused; otherwise, a snapshot is refused
+destination must exist and be pre-created with correct format and
+metadata. The file is then reused; otherwise, a snapshot is refused
 to avoid losing contents of the existing files.

 If I--quiesce is specified, libvirt will try to use guest agent
@@ -3233,8 +3235,9 @@ results in the following XML:

 If I--reuse-external is specified, and the domain XML or Idiskspec
 option requests an external snapshot with a destination of an existing
-file, then the destination must exist, and is reused; otherwise, a
-snapshot is refused to avoid losing contents of the 

[libvirt] [PATCHv2 0/2] doc: Improve snapshot/blockjob docs

2014-07-11 Thread Peter Krempa
Peter Krempa (2):
  doc: Document that snapshot name of block-backed disk isnt
autogenerated
  doc: Be more specific about semantics of _REUSE_EXT flag

 docs/formatsnapshot.html.in |  8 +---
 src/libvirt.c   | 23 ++-
 tools/virsh.pod | 21 +
 3 files changed, 32 insertions(+), 20 deletions(-)

-- 
2.0.0

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


[libvirt] [PATCH v3] LXC: add support for --config in setmem command

2014-07-11 Thread Chen Hanxiao
In lxc, we could not use setmem command
with --config options.
This patch will add support for this.

Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
v3: add max_balloon check for AFFECT_CONFIG

v2: use virDomainSetMemoryFlagsEnsureACL
remove redundant domain running check

 src/lxc/lxc_driver.c | 58 ++--
 1 file changed, 47 insertions(+), 11 deletions(-)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 3253211..f04b543 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -711,36 +711,64 @@ static int lxcDomainSetMaxMemory(virDomainPtr dom, 
unsigned long newmax)
 return ret;
 }
 
-static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem)
+static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
+   unsigned int flags)
 {
 virDomainObjPtr vm;
+virDomainDefPtr persistentDef = NULL;
+virCapsPtr caps = NULL;
 int ret = -1;
 virLXCDomainObjPrivatePtr priv;
+virLXCDriverPtr driver = dom-conn-privateData;
+virLXCDriverConfigPtr cfg = NULL;
+unsigned long oldmax = 0;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_DOMAIN_AFFECT_CONFIG, -1);
 
 if (!(vm = lxcDomObjFromDomain(dom)))
 goto cleanup;
 
+cfg = virLXCDriverGetConfig(driver);
+
 priv = vm-privateData;
 
-if (virDomainSetMemoryEnsureACL(dom-conn, vm-def)  0)
+if (virDomainSetMemoryFlagsEnsureACL(dom-conn, vm-def, flags)  0)
 goto cleanup;
 
-if (newmem  vm-def-mem.max_balloon) {
+if (!(caps = virLXCDriverGetCapabilities(driver, false)))
+goto cleanup;
+
+if (virDomainLiveConfigHelperMethod(caps, driver-xmlopt, vm, flags,
+persistentDef)  0)
+goto cleanup;
+
+if (flags  VIR_DOMAIN_AFFECT_LIVE)
+oldmax = vm-def-mem.max_balloon;
+if (flags  VIR_DOMAIN_AFFECT_CONFIG) {
+if (!oldmax || oldmax  persistentDef-mem.max_balloon)
+oldmax = persistentDef-mem.max_balloon;
+}
+
+if (newmem  oldmax) {
 virReportError(VIR_ERR_INVALID_ARG,
%s, _(Cannot set memory higher than max memory));
 goto cleanup;
 }
 
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   %s, _(Domain is not running));
-goto cleanup;
+if (flags  VIR_DOMAIN_AFFECT_LIVE) {
+if (virCgroupSetMemory(priv-cgroup, newmem)  0) {
+virReportError(VIR_ERR_OPERATION_FAILED,
+   %s, _(Failed to set memory for domain));
+goto cleanup;
+}
 }
 
-if (virCgroupSetMemory(priv-cgroup, newmem)  0) {
-virReportError(VIR_ERR_OPERATION_FAILED,
-   %s, _(Failed to set memory for domain));
-goto cleanup;
+if (flags  VIR_DOMAIN_AFFECT_CONFIG) {
+sa_assert(persistentDef);
+persistentDef-mem.cur_balloon = newmem;
+if (virDomainSaveConfig(cfg-configDir, persistentDef)  0)
+goto cleanup;
 }
 
 ret = 0;
@@ -748,9 +776,16 @@ static int lxcDomainSetMemory(virDomainPtr dom, unsigned 
long newmem)
  cleanup:
 if (vm)
 virObjectUnlock(vm);
+virObjectUnref(caps);
+virObjectUnref(cfg);
 return ret;
 }
 
+static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem)
+{
+return lxcDomainSetMemoryFlags(dom, newmem, VIR_DOMAIN_AFFECT_LIVE);
+}
+
 static int
 lxcDomainSetMemoryParameters(virDomainPtr dom,
  virTypedParameterPtr params,
@@ -5697,6 +5732,7 @@ static virDriver lxcDriver = {
 .domainGetMaxMemory = lxcDomainGetMaxMemory, /* 0.7.2 */
 .domainSetMaxMemory = lxcDomainSetMaxMemory, /* 0.7.2 */
 .domainSetMemory = lxcDomainSetMemory, /* 0.7.2 */
+.domainSetMemoryFlags = lxcDomainSetMemoryFlags, /* 1.2.7 */
 .domainSetMemoryParameters = lxcDomainSetMemoryParameters, /* 0.8.5 */
 .domainGetMemoryParameters = lxcDomainGetMemoryParameters, /* 0.8.5 */
 .domainSetBlkioParameters = lxcDomainSetBlkioParameters, /* 0.9.8 */
-- 
1.9.0

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


[libvirt] [PATCH v3 1/3] conf: Always format seclabel's model

2014-07-11 Thread Michal Privoznik
https://bugzilla.redhat.com/show_bug.cgi?id=1113860

We've always done that. Well, until 990e46c45. Point is, if we don't
format model, we may lose a domain on libvirtd restart. If the
seclabel is implicit however, we should skip it's formatting.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/conf/domain_conf.c | 34 +++---
 .../qemuxml2argv-seclabel-dynamic-none.xml | 28 ++
 tests/qemuxml2xmltest.c|  1 +
 3 files changed, 52 insertions(+), 11 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-none.xml

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b91ccf7..7b90903 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4614,8 +4614,23 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt,
 /* For the model 'none' none of the following labels is going to be
  * present. Hence, return now. */
 
-if (STREQ_NULLABLE(def-model, none))
+if (STREQ_NULLABLE(def-model, none)) {
+if (flags  VIR_DOMAIN_XML_INACTIVE) {
+/* Fix older configurations */
+def-type = VIR_DOMAIN_SECLABEL_NONE;
+def-relabel = false;
+} else {
+if (def-type != VIR_DOMAIN_SECLABEL_NONE) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(unsupported type='%s' to model 'none'),
+   virDomainSeclabelTypeToString(def-type));
+goto error;
+}
+/* combination of relabel='yes' and type='static'
+ * is checked a few lines above. */
+}
 return def;
+}
 
 /* Only parse label, if using static labels, or
  * if the 'live' VM XML is requested
@@ -14690,8 +14705,7 @@ virDomainEventActionDefFormat(virBufferPtr buf,
 
 static void
 virSecurityLabelDefFormat(virBufferPtr buf,
-  virSecurityLabelDefPtr def,
-  unsigned flags)
+  virSecurityLabelDefPtr def)
 {
 const char *sectype = virDomainSeclabelTypeToString(def-type);
 
@@ -14701,19 +14715,17 @@ virSecurityLabelDefFormat(virBufferPtr buf,
 if (def-type == VIR_DOMAIN_SECLABEL_DEFAULT)
 return;
 
-/* To avoid backward compatibility issues, suppress DAC labels that are
- * automatically generated.
+/* To avoid backward compatibility issues, suppress DAC and 'none' labels
+ * that are automatically generated.
  */
-if (STREQ_NULLABLE(def-model, dac)  def-implicit)
+if ((STREQ_NULLABLE(def-model, dac) ||
+ STREQ_NULLABLE(def-model, none))  def-implicit)
 return;
 
 virBufferAsprintf(buf, seclabel type='%s',
   sectype);
 
-/* When generating state XML do include the model */
-if (flags  VIR_DOMAIN_XML_INTERNAL_STATUS ||
-STRNEQ_NULLABLE(def-model, none))
-virBufferEscapeString(buf,  model='%s', def-model);
+virBufferEscapeString(buf,  model='%s', def-model);
 
 if (def-type == VIR_DOMAIN_SECLABEL_NONE) {
 virBufferAddLit(buf, /\n);
@@ -17923,7 +17935,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
 virBufferAddLit(buf, /devices\n);
 
 for (n = 0; n  def-nseclabels; n++)
-virSecurityLabelDefFormat(buf, def-seclabels[n], flags);
+virSecurityLabelDefFormat(buf, def-seclabels[n]);
 
 if (def-namespaceData  def-ns.format) {
 if ((def-ns.format)(buf, def-namespaceData)  0)
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-none.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-none.xml
new file mode 100644
index 000..cec59f8
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-none.xml
@@ -0,0 +1,28 @@
+domain type='qemu'
+  nameQEMUGuest1/name
+  uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid
+  memory unit='KiB'219100/memory
+  currentMemory unit='KiB'219100/currentMemory
+  vcpu placement='static' cpuset='1-4,8-20,525'1/vcpu
+  os
+type arch='i686' machine='pc'hvm/type
+boot dev='hd'/
+  /os
+  clock offset='utc'/
+  on_poweroffdestroy/on_poweroff
+  on_rebootrestart/on_reboot
+  on_crashdestroy/on_crash
+  devices
+emulator/usr/bin/qemu/emulator
+disk type='block' device='disk'
+  source dev='/dev/HostVG/QEMUGuest1'/
+  target dev='hda' bus='ide'/
+  address type='drive' controller='0' bus='0' target='0' unit='0'/
+/disk
+controller type='usb' index='0'/
+controller type='ide' index='0'/
+controller type='pci' index='0' model='pci-root'/
+memballoon model='virtio'/
+  /devices
+  seclabel type='none' model='none'/
+/domain
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 26e3cad..9f919de 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -307,6 +307,7 @@ mymain(void)
 DO_TEST_FULL(seclabel-static-labelskip, false, WHEN_ACTIVE);
 

[libvirt] [PATCH 3/3] virSecurityLabelDef: use enum type for @type

2014-07-11 Thread Michal Privoznik
There's this trend in libvirt of using enum types wherever possible.
Now that I'm at virSecurityLabelDef let's rework @type item of the
structure so we don't have to typecast it elsewhere.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/conf/domain_conf.c  | 6 --
 src/security/security_dac.c | 2 +-
 src/util/virseclabel.h  | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index de60cd2..b3a0ec8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4567,12 +4567,14 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt,
 p = virXPathStringLimit(string(./@type),
 VIR_SECURITY_LABEL_BUFLEN - 1, ctxt);
 if (p) {
-seclabel-type = virDomainSeclabelTypeFromString(p);
-if (seclabel-type = 0) {
+int type; /* virDomainSeclabelType */
+type = virDomainSeclabelTypeFromString(p);
+if (type = 0) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(invalid security type '%s'), p);
 goto error;
 }
+seclabel-type = type;
 }
 
 if (seclabel-type == VIR_DOMAIN_SECLABEL_STATIC ||
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 4d2a9d6..b4bfc57 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -1125,7 +1125,7 @@ virSecurityDACGenLabel(virSecurityManagerPtr mgr,
 return rc;
 }
 
-switch ((virDomainSeclabelType) seclabel-type) {
+switch (seclabel-type) {
 case VIR_DOMAIN_SECLABEL_STATIC:
 if (seclabel-label == NULL) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/util/virseclabel.h b/src/util/virseclabel.h
index 94c4dfc..abf9510 100644
--- a/src/util/virseclabel.h
+++ b/src/util/virseclabel.h
@@ -39,7 +39,7 @@ struct _virSecurityLabelDef {
 char *label;/* security label string */
 char *imagelabel;   /* security image label string */
 char *baselabel;/* base name of label string */
-int type;   /* virDomainSeclabelType */
+virDomainSeclabelType type; /* seclabel @type */
 bool relabel;   /* true (default) for allowing relabels */
 bool implicit;  /* true if seclabel is auto-added */
 };
-- 
1.8.5.5

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


[libvirt] [PATCHv3 0/3] Couple of seclabels improvements

2014-07-11 Thread Michal Privoznik
Some patches of the previous version are pushed already.
However, there are some new too.

Michal Privoznik (3):
  conf: Always format seclabel's model
  virSecurityLabelDefParseXML: Rework
  virSecurityLabelDef: use enum type for @type

 src/conf/domain_conf.c | 129 +++--
 src/security/security_dac.c|   2 +-
 src/util/virseclabel.h |   2 +-
 .../qemuxml2argv-seclabel-dynamic-none.xml |  28 +
 tests/qemuxml2xmltest.c|   1 +
 5 files changed, 101 insertions(+), 61 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-none.xml

-- 
1.8.5.5

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


[libvirt] [PATCH 2/3] virSecurityLabelDefParseXML: Rework

2014-07-11 Thread Michal Privoznik
Instead of allocating the virSecurityLabelDef structure ourselves, we
can utilize virSecurityLabelDefNew which even sets the default values
for us.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/conf/domain_conf.c | 103 -
 1 file changed, 50 insertions(+), 53 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7b90903..de60cd2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4553,91 +4553,87 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt,
 unsigned int flags)
 {
 char *p;
-virSecurityLabelDefPtr def = NULL;
+virSecurityLabelDefPtr seclabel = NULL;
 
-if (VIR_ALLOC(def)  0)
+p = virXPathStringLimit(string(./@model),
+VIR_SECURITY_MODEL_BUFLEN - 1, ctxt);
+
+if (!(seclabel = virSecurityLabelDefNew(p)))
 goto error;
 
+/* set seclabelault value */
+seclabel-type = VIR_DOMAIN_SECLABEL_DYNAMIC;
+
 p = virXPathStringLimit(string(./@type),
-VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
-if (p == NULL) {
-def-type = VIR_DOMAIN_SECLABEL_DYNAMIC;
-} else {
-def-type = virDomainSeclabelTypeFromString(p);
-VIR_FREE(p);
-if (def-type = 0) {
+VIR_SECURITY_LABEL_BUFLEN - 1, ctxt);
+if (p) {
+seclabel-type = virDomainSeclabelTypeFromString(p);
+if (seclabel-type = 0) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   %s, _(invalid security type));
+   _(invalid security type '%s'), p);
 goto error;
 }
 }
 
+if (seclabel-type == VIR_DOMAIN_SECLABEL_STATIC ||
+seclabel-type == VIR_DOMAIN_SECLABEL_NONE)
+seclabel-relabel = false;
+
+VIR_FREE(p);
 p = virXPathStringLimit(string(./@relabel),
 VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
-if (p != NULL) {
+if (p) {
 if (STREQ(p, yes)) {
-def-relabel = true;
+seclabel-relabel = true;
 } else if (STREQ(p, no)) {
-def-relabel = false;
+seclabel-relabel = false;
 } else {
 virReportError(VIR_ERR_XML_ERROR,
_(invalid security relabel value %s), p);
-VIR_FREE(p);
 goto error;
 }
-VIR_FREE(p);
-if (def-type == VIR_DOMAIN_SECLABEL_DYNAMIC 
-!def-relabel) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   %s, _(dynamic label type must use resource 
relabeling));
-goto error;
-}
-if (def-type == VIR_DOMAIN_SECLABEL_NONE 
-def-relabel) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   %s, _(resource relabeling is not compatible with 
'none' label type));
-goto error;
-}
-} else {
-if (def-type == VIR_DOMAIN_SECLABEL_STATIC ||
-def-type == VIR_DOMAIN_SECLABEL_NONE)
-def-relabel = false;
-else
-def-relabel = true;
 }
 
-/* Always parse model */
-p = virXPathStringLimit(string(./@model),
-VIR_SECURITY_MODEL_BUFLEN-1, ctxt);
-def-model = p;
+if (seclabel-type == VIR_DOMAIN_SECLABEL_DYNAMIC 
+!seclabel-relabel) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   %s, _(dynamic label type must use resource 
relabeling));
+goto error;
+}
+if (seclabel-type == VIR_DOMAIN_SECLABEL_NONE 
+seclabel-relabel) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   %s, _(resource relabeling is not compatible with 
'none' label type));
+goto error;
+}
 
 /* For the model 'none' none of the following labels is going to be
  * present. Hence, return now. */
 
-if (STREQ_NULLABLE(def-model, none)) {
+if (STREQ_NULLABLE(seclabel-model, none)) {
 if (flags  VIR_DOMAIN_XML_INACTIVE) {
 /* Fix older configurations */
-def-type = VIR_DOMAIN_SECLABEL_NONE;
-def-relabel = false;
+seclabel-type = VIR_DOMAIN_SECLABEL_NONE;
+seclabel-relabel = false;
 } else {
-if (def-type != VIR_DOMAIN_SECLABEL_NONE) {
+if (seclabel-type != VIR_DOMAIN_SECLABEL_NONE) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(unsupported type='%s' to model 'none'),
-   virDomainSeclabelTypeToString(def-type));
+   virDomainSeclabelTypeToString(seclabel-type));
 goto error;
 }
 /* combination of relabel='yes' and type='static'
  * is checked a few lines above. */
 }
-return def;
+return seclabel;
 }
 
 

[libvirt] [PATCH] schema: pool: netfs: Don't enforce slash in glusterfs pool source

2014-07-11 Thread Peter Krempa
Gluster volumes don't start with a leading slash. Our schema for netfs
gluster pools enforces it though. Luckily mount.glusterfs skips it.
Allow a slashless volume name for glusterfs netfs mounts in the schema.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1101999
---
 docs/schemas/basictypes.rng|  6 +++
 docs/schemas/storagepool.rng   | 44 +-
 .../pool-netfs-gluster-without-slash.xml   | 12 ++
 3 files changed, 53 insertions(+), 9 deletions(-)
 create mode 100644 
tests/storagepoolxml2xmlin/pool-netfs-gluster-without-slash.xml

diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
index 5fe3a97..9c9419f 100644
--- a/docs/schemas/basictypes.rng
+++ b/docs/schemas/basictypes.rng
@@ -231,6 +231,12 @@
 /data
   /define

+  define name=dirPath
+data type=string
+  param 
name=pattern[a-zA-Z0-9_\.\+\-\\amp;quot;apos;lt;gt;/%]*/param
+/data
+  /define
+
   define name=absFilePath
 data type=string
   param 
name=pattern/[a-zA-Z0-9_\.\+\-\\amp;quot;apos;lt;gt;/%,]+/param
diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
index 8d7a94d..b2d1473 100644
--- a/docs/schemas/storagepool.rng
+++ b/docs/schemas/storagepool.rng
@@ -327,6 +327,15 @@
 /element
   /define

+  define name='sourceinfonetfsgluster'
+element name='dir'
+  attribute name='path'
+ref name='dirPath'/
+  /attribute
+  empty/
+/element
+  /define
+
   define name='sourceinfoname'
 element name='name'
   text/
@@ -394,7 +403,6 @@
 valueauto/value
 valuenfs/value
 valuecifs/value
-valueglusterfs/value
   /choice
 /attribute
   /element
@@ -468,14 +476,32 @@

   define name='sourcenetfs'
 element name='source'
-  interleave
-ref name='sourceinfohost'/
-ref name='sourceinfodir'/
-ref name='sourcefmtnetfs'/
-optional
-  ref name='sourceinfovendor'/
-/optional
-  /interleave
+  choice
+group
+  interleave
+ref name='sourceinfohost'/
+ref name='sourceinfodir'/
+ref name='sourcefmtnetfs'/
+optional
+ref name='sourceinfovendor'/
+/optional
+  /interleave
+/group
+group
+  interleave
+ref name='sourceinfohost'/
+ref name='sourceinfonetfsgluster'/
+element name='format'
+  attribute name='type'
+valueglusterfs/value
+  /attribute
+/element
+optional
+ref name='sourceinfovendor'/
+/optional
+  /interleave
+/group
+  /choice
 /element
   /define

diff --git a/tests/storagepoolxml2xmlin/pool-netfs-gluster-without-slash.xml 
b/tests/storagepoolxml2xmlin/pool-netfs-gluster-without-slash.xml
new file mode 100644
index 000..69a2c6d
--- /dev/null
+++ b/tests/storagepoolxml2xmlin/pool-netfs-gluster-without-slash.xml
@@ -0,0 +1,12 @@
+pool type='netfs'
+  source
+host name='example.com'/
+format type='glusterfs'/
+dir path='volume'/
+  /source
+  namenetfs-gluster/name
+  uuidd5609ced-94b1-489e-b218-eff35c30336a/uuid
+  target
+path/mnt/gluster/path
+  /target
+/pool
-- 
2.0.0

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


Re: [libvirt] [PATCH v2 0/4] Couple of seclabels improvements

2014-07-11 Thread John Ferlan


On 07/10/2014 10:04 AM, Michal Privoznik wrote:
 diff to v1:
 - rework the 3rd patch
 - introduce one more bugfix
 
 Michal Privoznik (4):
   virSecurityLabelDef: substitute 'norelabel' with 'relabel'
   virSecurityDeviceLabelDef: substitute 'norelabel' with 'relabel'
   conf: Always format seclabel's model
   conf: Don't allow multiple seclabels for same model
 
  src/conf/domain_conf.c | 67 
 --
  src/security/security_apparmor.c   | 10 ++--
  src/security/security_dac.c| 22 +++
  src/security/security_manager.c|  2 +-
  src/security/security_selinux.c| 32 +--
  src/util/virseclabel.c |  2 +-
  src/util/virseclabel.h |  4 +-
  .../qemuxml2argv-seclabel-dynamic-none.xml | 28 +
  .../qemuxml2argv-seclabel-multiple.xml | 40 +
  tests/qemuxml2argvtest.c   |  1 +
  tests/qemuxml2xmltest.c|  1 +
  11 files changed, 142 insertions(+), 67 deletions(-)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-none.xml
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-multiple.xml
 

There's a Coverity issue from these patches - it looks like perhaps
patch 12 were combined when submitted into commit id '13adf1b' which has:

virSecurityLabelDefPtr
 virSecurityLabelDefNew(const char *model)
 {
 virSecurityLabelDefPtr seclabel = NULL;

 if (VIR_ALLOC(seclabel)  0 ||
 VIR_STRDUP(seclabel-model, model)  0) {
 virSecurityLabelDefFree(seclabel);
 seclabel = NULL;
 }

+seclabel-relabel = true;
+
 return seclabel;
 }


See the problem at all?  It's a FORWARD_NULL on 'seclabel'.

John

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


Re: [libvirt] [PATCH 1/4] vhost-user support: domain configuration

2014-07-11 Thread Michele Paolino

On 10/07/2014 18:01, Michal Privoznik wrote:

On 02.07.2014 15:20, Michele Paolino wrote:

vhost-user is added as a virDomainChrSourceDefPtr variable in the
virtual network interface configuration structure.
This patch adds support to parse vhost-user element. The XML file
looks like:

interface type='vhostuser'
  source type='unix' path='/tmp/vhost.sock' mode='server'/
  mac address='52:54:00:3b:83:1a'/
  model type='virtio'/
/interface

Signed-off-by: Michele Paolino m.paol...@virtualopensystems.com
---
   src/conf/domain_conf.c | 81 
++
   src/conf/domain_conf.h | 10 +--
   2 files changed, 89 insertions(+), 2 deletions(-)

Introducing a new device (subtype of a device) must always go hand in hand with 
documentation and XML schema adjustment. Moreover, it would be nice to test the 
new XML (e.g. domainschematest or qemuxml2argv stub (a .xml file under 
tests/qemuxml2argvdata without .args counterpart which will be introduced once 
qemu implementation is done)).


Yes, these files are present in 3/4. I will post the v2 of this series 
in a single patch.





diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ea09bdc..de52ca4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -349,6 +349,7 @@ VIR_ENUM_IMPL(virDomainFSWrpolicy, 
VIR_DOMAIN_FS_WRPOLICY_LAST,
   VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST,
 user,
 ethernet,
+  vhostuser,
 server,
 client,
 mcast,
@@ -1361,6 +1362,10 @@ void virDomainNetDefFree(virDomainNetDefPtr def)
   VIR_FREE(def-data.ethernet.ipaddr);
   break;
   
+case VIR_DOMAIN_NET_TYPE_VHOSTUSER:

+virDomainChrSourceDefFree(def-data.vhostuser);
+break;
+
   case VIR_DOMAIN_NET_TYPE_SERVER:
   case VIR_DOMAIN_NET_TYPE_CLIENT:
   case VIR_DOMAIN_NET_TYPE_MCAST:
@@ -6665,6 +6670,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
   char *mode = NULL;
   char *linkstate = NULL;
   char *addrtype = NULL;
+char *vhostuser_mode = NULL;
+char *vhostuser_path = NULL;
+char *vhostuser_type = NULL;
   virNWFilterHashTablePtr filterparams = NULL;
   virDomainActualNetDefPtr actual = NULL;
   xmlNodePtr oldnode = ctxt-node;
@@ -6710,6 +6718,21 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
  xmlStrEqual(cur-name, BAD_CAST source)) {
   dev  = virXMLPropString(cur, dev);
   mode = virXMLPropString(cur, mode);
+} else if (!vhostuser_path  !vhostuser_mode  !vhostuser_type
+def-type == VIR_DOMAIN_NET_TYPE_VHOSTUSER 
+   xmlStrEqual(cur-name, BAD_CAST source)) {
+vhostuser_type = virXMLPropString(cur, type);
+if (!vhostuser_type || STREQ(vhostuser_type, unix)) {
+vhostuser_path = virXMLPropString(cur, path);
+vhostuser_mode = virXMLPropString(cur, mode);
+}
+else {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(type='%s' unsupported for
+interface type='vhostuser'),
+   vhostuser_type);
+goto error;

I'd move this check a few lines below to the other checks.


The check has been done here because if in future we will decide to 
support other chardevs in addition to the unix socket, the XML file will 
be different.


For example, if we want to add the support for a TCP socket, the path 
attribute needs to be replaced with host, service and protocol. After a 
quick look at the _virDomainChrSourceDef structure, the XML description 
in this case should look like:


   source type='tcp' host='host' service='serv' mode='mode' 
protocol='prot'/


Do we want to add these additional checks only when the support for 
other chardevs will be added, or is there an alternative solution?





+}
   } else if (!def-virtPortProfile
   xmlStrEqual(cur-name, BAD_CAST virtualport)) {
   if (def-type == VIR_DOMAIN_NET_TYPE_NETWORK) {
@@ -6867,6 +6890,50 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
   actual = NULL;
   break;
   
+case VIR_DOMAIN_NET_TYPE_VHOSTUSER:

+if (!model || STRNEQ(model, virtio)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Wrong or no model 'type' attribute 
+   specified with interface type='vhostuser'/. 
+   vhostuser requires the virtio-net* frontend));
+goto error;
+}
+
+if (VIR_ALLOC(def-data.vhostuser)  0)
+goto error;
+
+if (STREQ(vhostuser_type, unix)) {

Ouch, in the code I've commented above, 

Re: [libvirt] [PATCH 3/4] vhost-user support: tests and docs

2014-07-11 Thread Michele Paolino

On 10/07/2014 18:01, Michal Privoznik wrote:

On 02.07.2014 15:20, Michele Paolino wrote:

This patch adds the test files and the documentation for vhost-user.

Signed-off-by: Michele Paolino m.paol...@virtualopensystems.com
---
  docs/formatdomain.html.in  | 34 
+++
  docs/schemas/domaincommon.rng  | 39 
++

  .../qemuxml2argv-net-vhostuser.args|  7 
  .../qemuxml2argv-net-vhostuser.xml | 33 
++

  tests/qemuxml2argvtest.c   |  1 +
  tests/qemuxml2xmltest.c|  1 +
  6 files changed, 115 insertions(+)
  create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args
  create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.xml




Aha! You are doing here what I've just requested in 1/4. Still I 
rather see it in a single patch (even though it will be bigger) 
because it make maintainers life easier. You know, backporting a 
single patch versus digging through 'git log' to search for this 
commit ...



diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 3f8bbee..606b7d4 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3927,6 +3927,40 @@ qemu-kvm -net nic,model=? /dev/null
span class=sinceSince 0.9.5/span
  /p

+h5a name=elementVhostuservhost-user interface/a/h5
+
+p
+ vhost-user enables the communication between a QEMU virtual 
machine

+ and other userspace process using the Virtio transport protocol.
+ A char dev (e.g. Unix socket) is used for the control plane, while
+ the data plane is based on shared memory.
+/p
+
+pre
+  ...
+  lt;devicesgt;
+lt;interface type='vhostuser'gt;
+  lt;source type='unix' path='/tmp/vhost.sock' mode='server'gt;
+  lt;/sourcegt;
+  lt;mac address='52:54:00:3b:83:1a'gt;
+  lt;/macgt;
+  lt;model type='virtio'gt;
+  lt;/modelgt;
+lt;/interfacegt;
+  lt;/devicesgt;
+  .../pre
+
+p
+  The codelt;sourcegt;/code element has to be specified
+  along with the type of char device.
+  Currently, only type='unix' is supported, where the path (the
+  directory path of the socket) and mode attributes are required.
+  Both codemode='server'/code and codemode='client'/code
+  are supported.
+  vhost-user requires the virtio model type, thus the
+  codelt;modelgt;/code element is mandatory.
+/p
+
  h4a name=elementsInputInput devices/a/h4

  p
diff --git a/docs/schemas/domaincommon.rng 
b/docs/schemas/domaincommon.rng

index af51eee..f85dd61 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1968,6 +1968,45 @@
  /group
  group
attribute name=type
+valuevhostuser/value
+  /attribute
+  interleave
+optional
+  element name=source


I wouldn't say source/ is optional in case of interface 
type='vhostnet'/. It contains crucial information that helps us 
construct the correct qemu command line which would not be possible 
otherwise.


I agree, but the schemas of the other interfaces it's the same. Please 
see the Laine's comment at 
http://www.redhat.com/archives/libvir-list/2014-June/msg00279.html





+attribute name=type
+  choice
+valuenull/value
+valuevc/value
+valuepty/value
+valuedev/value
+valuefile/value
+valuepipe/value
+valuestdio/value
+valueudp/value
+valuetcp/value
+valueunix/value
+valuespicevmc/value
+valuespiceport/value
+valuenmdm/value


Honestly, I'm inclined to not enumerate all of these here now. I mean, 
only 'unix' is supported now. On the other hand, it's acceptable to 
have a RNG schema that allows something more than our XML parser.



+  /choice
+/attribute
+attribute name=path
+  ref name=absFilePath/
+/attribute
+attribute name=mode
+  choice
+valueserver/value
+valueclient/value
+  /choice
+/attribute
+empty/
+  /element
+/optional
+ref name=interface-options/
+  /interleave
+/group
+group
+  attribute name=type
  valuenetwork/value
/attribute
interleave
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args 
b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args

new file mode 100644
index 000..cc66ec3
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args
@@ 

Re: [libvirt] [PATCH 3/4] vhost-user support: tests and docs

2014-07-11 Thread Michal Privoznik

On 11.07.2014 13:07, Michele Paolino wrote:

On 10/07/2014 18:01, Michal Privoznik wrote:

On 02.07.2014 15:20, Michele Paolino wrote:

This patch adds the test files and the documentation for vhost-user.

Signed-off-by: Michele Paolino m.paol...@virtualopensystems.com
---
  docs/formatdomain.html.in  | 34
+++
  docs/schemas/domaincommon.rng  | 39
++
  .../qemuxml2argv-net-vhostuser.args|  7 
  .../qemuxml2argv-net-vhostuser.xml | 33
++
  tests/qemuxml2argvtest.c   |  1 +
  tests/qemuxml2xmltest.c|  1 +
  6 files changed, 115 insertions(+)
  create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args
  create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.xml




diff --git a/docs/schemas/domaincommon.rng
b/docs/schemas/domaincommon.rng
index af51eee..f85dd61 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1968,6 +1968,45 @@
  /group
  group
attribute name=type
+valuevhostuser/value
+  /attribute
+  interleave
+optional
+  element name=source


I wouldn't say source/ is optional in case of interface
type='vhostnet'/. It contains crucial information that helps us
construct the correct qemu command line which would not be possible
otherwise.


I agree, but the schemas of the other interfaces it's the same. Please
see the Laine's comment at
http://www.redhat.com/archives/libvir-list/2014-June/msg00279.html


Well, not for all other interface types. Moreover, currently the your 
patches require source/. But I can live with schema wider than our parser.


Michal

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


Re: [libvirt] [PATCH 1/4] vhost-user support: domain configuration

2014-07-11 Thread Michal Privoznik

On 11.07.2014 13:06, Michele Paolino wrote:

On 10/07/2014 18:01, Michal Privoznik wrote:

On 02.07.2014 15:20, Michele Paolino wrote:

vhost-user is added as a virDomainChrSourceDefPtr variable in the
virtual network interface configuration structure.
This patch adds support to parse vhost-user element. The XML file
looks like:

interface type='vhostuser'
  source type='unix' path='/tmp/vhost.sock' mode='server'/
  mac address='52:54:00:3b:83:1a'/
  model type='virtio'/
/interface

Signed-off-by: Michele Paolino m.paol...@virtualopensystems.com
---
   src/conf/domain_conf.c | 81
++
   src/conf/domain_conf.h | 10 +--
   2 files changed, 89 insertions(+), 2 deletions(-)

Introducing a new device (subtype of a device) must always go hand in
hand with documentation and XML schema adjustment. Moreover, it would
be nice to test the new XML (e.g. domainschematest or qemuxml2argv
stub (a .xml file under tests/qemuxml2argvdata without .args
counterpart which will be introduced once qemu implementation is done)).


Yes, these files are present in 3/4. I will post the v2 of this series
in a single patch.




diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ea09bdc..de52ca4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -349,6 +349,7 @@ VIR_ENUM_IMPL(virDomainFSWrpolicy,
VIR_DOMAIN_FS_WRPOLICY_LAST,
   VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST,
 user,
 ethernet,
+  vhostuser,
 server,
 client,
 mcast,
@@ -1361,6 +1362,10 @@ void virDomainNetDefFree(virDomainNetDefPtr def)
   VIR_FREE(def-data.ethernet.ipaddr);
   break;
+case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+virDomainChrSourceDefFree(def-data.vhostuser);
+break;
+
   case VIR_DOMAIN_NET_TYPE_SERVER:
   case VIR_DOMAIN_NET_TYPE_CLIENT:
   case VIR_DOMAIN_NET_TYPE_MCAST:
@@ -6665,6 +6670,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr
xmlopt,
   char *mode = NULL;
   char *linkstate = NULL;
   char *addrtype = NULL;
+char *vhostuser_mode = NULL;
+char *vhostuser_path = NULL;
+char *vhostuser_type = NULL;
   virNWFilterHashTablePtr filterparams = NULL;
   virDomainActualNetDefPtr actual = NULL;
   xmlNodePtr oldnode = ctxt-node;
@@ -6710,6 +6718,21 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr
xmlopt,
  xmlStrEqual(cur-name, BAD_CAST source)) {
   dev  = virXMLPropString(cur, dev);
   mode = virXMLPropString(cur, mode);
+} else if (!vhostuser_path  !vhostuser_mode 
!vhostuser_type
+def-type == VIR_DOMAIN_NET_TYPE_VHOSTUSER 
+   xmlStrEqual(cur-name, BAD_CAST source)) {
+vhostuser_type = virXMLPropString(cur, type);
+if (!vhostuser_type || STREQ(vhostuser_type, unix)) {
+vhostuser_path = virXMLPropString(cur, path);
+vhostuser_mode = virXMLPropString(cur, mode);
+}
+else {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(type='%s' unsupported for
+interface type='vhostuser'),
+   vhostuser_type);
+goto error;

I'd move this check a few lines below to the other checks.


The check has been done here because if in future we will decide to
support other chardevs in addition to the unix socket, the XML file will
be different.


Until then I find the code easier to read with checks grouped.



For example, if we want to add the support for a TCP socket, the path
attribute needs to be replaced with host, service and protocol. After a
quick look at the _virDomainChrSourceDef structure, the XML description
in this case should look like:

source type='tcp' host='host' service='serv' mode='mode'
protocol='prot'/

Do we want to add these additional checks only when the support for
other chardevs will be added, or is there an alternative solution?


Yes. There's no need to introduce the checks for unsupported 
combinations now.







+}
   } else if (!def-virtPortProfile
   xmlStrEqual(cur-name, BAD_CAST
virtualport)) {
   if (def-type == VIR_DOMAIN_NET_TYPE_NETWORK) {
@@ -6867,6 +6890,50 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr
xmlopt,
   actual = NULL;
   break;
+case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+if (!model || STRNEQ(model, virtio)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Wrong or no model 'type' attribute 
+   specified with interface
type='vhostuser'/. 
+   vhostuser requires the virtio-net*
frontend));
+goto error;
+ 

Re: [libvirt] [PATCH] leaseshelper: add enhancements to support all events

2014-07-11 Thread Nehal J Wani
On Fri, Jul 11, 2014 at 5:49 AM, Nehal J Wani nehaljw.k...@gmail.com wrote:
 This patch enables the helper program to detect event(s) triggered when there
 is a change in lease length or expiry and client-id. This transfers complete
 control of leases database to libvirt and suppresses use of the lease database
 file (network-name.leases). That file will not be created, read, or written.
 This is achieved by adding the option --leasefile-ro to dnsmasq and applying
 the symlink technique, which helps us map events related to leases with their
 corresponding network bridges.
 Example:
 /var/lib/libvirt/dnsmasq/virbr0.helper - 
 /home/wani/libvirt/src/libvirt_leaseshelper
 /var/lib/libvirt/dnsmasq/virbr3.helper - 
 /home/wani/libvirt/src/libvirt_leaseshelper

 Also, this requires the addition of a new non-lease entry in our custom lease
 database: server-duid. It is required to identify a DHCPv6 server.

 Now that dnsmasq doesn't maintain its own leases database, it relies on our
 helper program to tell it about previous leases and server duid. Thus, this
 patch makes our leases program honor an extra action: init, in which it 
 sends
 the known info in a particular format to dnsmasq by printing it to stdout.

 ---
  src/network/bridge_driver.c |  43 +++-
  src/network/leaseshelper.c  | 156 
 +---
  2 files changed, 175 insertions(+), 24 deletions(-)

make syntax-check reported two flaws. Following diff rectifies it:

diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c
index 3d6c773..bdc1b77 100644
--- a/src/network/leaseshelper.c
+++ b/src/network/leaseshelper.c
@@ -30,6 +30,7 @@
 #include stdlib.h
 #include sys/stat.h

+#include dirname.h
 #include virutil.h
 #include virthread.h
 #include virfile.h
@@ -54,7 +55,7 @@
  * Use this when passing possibly-NULL strings to printf-a-likes.
  * Required for unkown parameters during init call.
  */
-# define EMPTY_STR(s) ((s) ? (s) : *)
+#define EMPTY_STR(s) ((s) ? (s) : *)


 static const char *program_name;
@@ -181,7 +182,7 @@ main(int argc, char **argv)
  * expired leases. Our symlink hack provides us an alternative method to
  * find it */
 if (!interface) {
-interface = basename(argv[0]);
+interface = last_component(argv[0]);
*(strchr(interface, '.')) = '\0';
 }





Regards,
Nehal J Wani

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


Re: [libvirt] [PATCH v3] LXC: add support for --config in setmem command

2014-07-11 Thread Ján Tomko
On 07/11/2014 11:26 AM, Chen Hanxiao wrote:
 In lxc, we could not use setmem command
 with --config options.
 This patch will add support for this.
 
 Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
 ---
 v3: add max_balloon check for AFFECT_CONFIG
 
 v2: use virDomainSetMemoryFlagsEnsureACL
 remove redundant domain running check
 
  src/lxc/lxc_driver.c | 58 
 ++--
  1 file changed, 47 insertions(+), 11 deletions(-)

ACK and pushed.

Jan



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

[libvirt] [PATCHv2 0/2] storage: Don't wipe volumes on remote filesystems with local tools

2014-07-11 Thread Peter Krempa
Version 2 now splits the stuff into separate driver backend funcs.

Peter Krempa (2):
  storage: wipe: Move helper code into storage backend
  storage: Split out volume wiping as separate backend function

 src/storage/storage_backend.c | 203 +
 src/storage/storage_backend.h |  12 ++
 src/storage/storage_backend_disk.c|   1 +
 src/storage/storage_backend_fs.c  |   3 +
 src/storage/storage_backend_iscsi.c   |   1 +
 src/storage/storage_backend_logical.c |   1 +
 src/storage/storage_backend_mpath.c   |   1 +
 src/storage/storage_backend_scsi.c|   1 +
 src/storage/storage_driver.c  | 205 +-
 9 files changed, 229 insertions(+), 199 deletions(-)

-- 
2.0.0

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


[libvirt] [PATCHv2 2/2] storage: Split out volume wiping as separate backend function

2014-07-11 Thread Peter Krempa
For non-local storage drivers we can't expect to use the scrub tool to
wipe the volume. Split the code into a separate backend function so that
we can add protocol specific code later.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1118710
---
 src/storage/storage_backend.h |  6 ++
 src/storage/storage_backend_disk.c|  1 +
 src/storage/storage_backend_fs.c  |  3 +++
 src/storage/storage_backend_iscsi.c   |  1 +
 src/storage/storage_backend_logical.c |  1 +
 src/storage/storage_backend_mpath.c   |  1 +
 src/storage/storage_backend_scsi.c|  1 +
 src/storage/storage_driver.c  | 10 +++---
 8 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 5e251d7..e48da5b 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -87,6 +87,11 @@ typedef int (*virStorageBackendVolumeUpload)(virConnectPtr 
conn,
  unsigned long long offset,
  unsigned long long len,
  unsigned int flags);
+typedef int (*virStorageBackendVolumeWipe)(virConnectPtr conn,
+   virStoragePoolObjPtr pool,
+   virStorageVolDefPtr vol,
+   unsigned int algorithm,
+   unsigned int flags);

 /* File creation/cloning functions used for cloning between backends */
 int virStorageBackendCreateRaw(virConnectPtr conn,
@@ -150,6 +155,7 @@ struct _virStorageBackend {
 virStorageBackendVolumeResize resizeVol;
 virStorageBackendVolumeUpload uploadVol;
 virStorageBackendVolumeDownload downloadVol;
+virStorageBackendVolumeWipe wipeVol;
 };

 virStorageBackendPtr virStorageBackendForType(int type);
diff --git a/src/storage/storage_backend_disk.c 
b/src/storage/storage_backend_disk.c
index f900dee..d85f13f 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -794,4 +794,5 @@ virStorageBackend virStorageBackendDisk = {
 .buildVolFrom = virStorageBackendDiskBuildVolFrom,
 .uploadVol = virStorageBackendVolUploadLocal,
 .downloadVol = virStorageBackendVolDownloadLocal,
+.wipeVol = virStorageBackendVolWipeLocal,
 };
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 1615c12..58e849f 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1291,6 +1291,7 @@ virStorageBackend virStorageBackendDirectory = {
 .resizeVol = virStorageBackendFileSystemVolResize,
 .uploadVol = virStorageBackendVolUploadLocal,
 .downloadVol = virStorageBackendVolDownloadLocal,
+.wipeVol = virStorageBackendVolWipeLocal,
 };

 #if WITH_STORAGE_FS
@@ -1311,6 +1312,7 @@ virStorageBackend virStorageBackendFileSystem = {
 .resizeVol = virStorageBackendFileSystemVolResize,
 .uploadVol = virStorageBackendVolUploadLocal,
 .downloadVol = virStorageBackendVolDownloadLocal,
+.wipeVol = virStorageBackendVolWipeLocal,
 };
 virStorageBackend virStorageBackendNetFileSystem = {
 .type = VIR_STORAGE_POOL_NETFS,
@@ -1330,6 +1332,7 @@ virStorageBackend virStorageBackendNetFileSystem = {
 .resizeVol = virStorageBackendFileSystemVolResize,
 .uploadVol = virStorageBackendVolUploadLocal,
 .downloadVol = virStorageBackendVolDownloadLocal,
+.wipeVol = virStorageBackendVolWipeLocal,
 };


diff --git a/src/storage/storage_backend_iscsi.c 
b/src/storage/storage_backend_iscsi.c
index 7345571..1d0cf73 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -476,4 +476,5 @@ virStorageBackend virStorageBackendISCSI = {
 .findPoolSources = virStorageBackendISCSIFindPoolSources,
 .uploadVol = virStorageBackendVolUploadLocal,
 .downloadVol = virStorageBackendVolDownloadLocal,
+.wipeVol = virStorageBackendVolWipeLocal,
 };
diff --git a/src/storage/storage_backend_logical.c 
b/src/storage/storage_backend_logical.c
index faa9a4b..562f019 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -843,4 +843,5 @@ virStorageBackend virStorageBackendLogical = {
 .deleteVol = virStorageBackendLogicalDeleteVol,
 .uploadVol = virStorageBackendVolUploadLocal,
 .downloadVol = virStorageBackendVolDownloadLocal,
+.wipeVol = virStorageBackendVolWipeLocal,
 };
diff --git a/src/storage/storage_backend_mpath.c 
b/src/storage/storage_backend_mpath.c
index 402faa0..f21ae4c 100644
--- a/src/storage/storage_backend_mpath.c
+++ b/src/storage/storage_backend_mpath.c
@@ -289,4 +289,5 @@ virStorageBackend virStorageBackendMpath = {
 .refreshPool = virStorageBackendMpathRefreshPool,
 .uploadVol = virStorageBackendVolUploadLocal,
 .downloadVol = virStorageBackendVolDownloadLocal,
+.wipeVol = 

[libvirt] [PATCHv2 1/2] storage: wipe: Move helper code into storage backend

2014-07-11 Thread Peter Krempa
The next patch will move the storage volume wiping code into the
individual backends. This patch splits out the common code to wipe a
local volume into a separate backend helper so that the next patch is
simpler.
---
 src/storage/storage_backend.c | 203 ++
 src/storage/storage_backend.h |   6 ++
 src/storage/storage_driver.c  | 199 +
 3 files changed, 210 insertions(+), 198 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 7b17ca4..8e62403 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1700,6 +1700,209 @@ virStorageBackendVolDownloadLocal(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 return virFDStreamOpenFile(stream, vol-target.path, offset, len, 
O_RDONLY);
 }

+
+/* If the volume we're wiping is already a sparse file, we simply
+ * truncate and extend it to its original size, filling it with
+ * zeroes.  This behavior is guaranteed by POSIX:
+ *
+ * http://www.opengroup.org/onlinepubs/9699919799/functions/ftruncate.html
+ *
+ * If fildes refers to a regular file, the ftruncate() function shall
+ * cause the size of the file to be truncated to length. If the size
+ * of the file previously exceeded length, the extra data shall no
+ * longer be available to reads on the file. If the file previously
+ * was smaller than this size, ftruncate() shall increase the size of
+ * the file. If the file size is increased, the extended area shall
+ * appear as if it were zero-filled.
+ */
+static int
+virStorageBackendVolZeroSparseFileLocal(virStorageVolDefPtr vol,
+off_t size,
+int fd)
+{
+int ret = -1;
+
+ret = ftruncate(fd, 0);
+if (ret == -1) {
+virReportSystemError(errno,
+ _(Failed to truncate volume with 
+   path '%s' to 0 bytes),
+ vol-target.path);
+return ret;
+}
+
+ret = ftruncate(fd, size);
+if (ret == -1) {
+virReportSystemError(errno,
+ _(Failed to truncate volume with 
+   path '%s' to %ju bytes),
+ vol-target.path, (uintmax_t)size);
+}
+
+return ret;
+}
+
+
+static int
+virStorageBackendWipeExtentLocal(virStorageVolDefPtr vol,
+ int fd,
+ off_t extent_start,
+ off_t extent_length,
+ char *writebuf,
+ size_t writebuf_length,
+ size_t *bytes_wiped)
+{
+int ret = -1, written = 0;
+off_t remaining = 0;
+size_t write_size = 0;
+
+VIR_DEBUG(extent logical start: %ju len: %ju,
+  (uintmax_t)extent_start, (uintmax_t)extent_length);
+
+if ((ret = lseek(fd, extent_start, SEEK_SET))  0) {
+virReportSystemError(errno,
+ _(Failed to seek to position %ju in volume 
+   with path '%s'),
+ (uintmax_t)extent_start, vol-target.path);
+goto cleanup;
+}
+
+remaining = extent_length;
+while (remaining  0) {
+
+write_size = (writebuf_length  remaining) ? writebuf_length : 
remaining;
+written = safewrite(fd, writebuf, write_size);
+if (written  0) {
+virReportSystemError(errno,
+ _(Failed to write %zu bytes to 
+   storage volume with path '%s'),
+ write_size, vol-target.path);
+
+goto cleanup;
+}
+
+*bytes_wiped += written;
+remaining -= written;
+}
+
+if (fdatasync(fd)  0) {
+ret = -errno;
+virReportSystemError(errno,
+ _(cannot sync data to volume with path '%s'),
+ vol-target.path);
+goto cleanup;
+}
+
+VIR_DEBUG(Wrote %zu bytes to volume with path '%s',
+  *bytes_wiped, vol-target.path);
+
+ret = 0;
+
+ cleanup:
+return ret;
+}
+
+
+int
+virStorageBackendVolWipeLocal(virConnectPtr conn ATTRIBUTE_UNUSED,
+  virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
+  virStorageVolDefPtr vol,
+  unsigned int algorithm,
+  unsigned int flags)
+{
+int ret = -1, fd = -1;
+struct stat st;
+char *writebuf = NULL;
+size_t bytes_wiped = 0;
+virCommandPtr cmd = NULL;
+
+virCheckFlags(0, -1);
+
+VIR_DEBUG(Wiping volume with path '%s' and algorithm %u,
+  vol-target.path, algorithm);
+
+fd = open(vol-target.path, O_RDWR);
+if (fd == -1) {
+virReportSystemError(errno,
+ _(Failed to open 

Re: [libvirt] CPU model API (v2)

2014-07-11 Thread Zeeshan Ali (Khattak)
On Thu, Jul 10, 2014 at 9:47 PM, Zeeshan Ali (Khattak)
zeesha...@gnome.org wrote:
 v2:

 * Correct hierarchy for GVirConfigDomainCpuModel
 * Correct order of new symbols in .sym file

Ouch, I messed-up the git-send-email commandline and ended-up without
the 'libvirt-glib' prefix to subject lines. Sorry for that.


-- 
Regards,

Zeeshan Ali (Khattak)

Befriend GNOME: http://www.gnome.org/friends/

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


[libvirt] [PATCH 3/8] hostdev: Introduce virDomainHostdevSubsysSCSI

2014-07-11 Thread John Ferlan
Create a separate typedef for the hostdev union data describing SCSI
Then adjust the code to use the new pointer

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/conf/domain_audit.c  |  7 +++---
 src/conf/domain_conf.c   | 40 --
 src/conf/domain_conf.h   | 18 --
 src/qemu/qemu_cgroup.c   |  7 +++---
 src/qemu/qemu_command.c  |  7 +++---
 src/qemu/qemu_conf.c | 18 --
 src/qemu/qemu_hotplug.c  | 12 -
 src/security/security_apparmor.c | 10 +++-
 src/security/security_dac.c  | 20 ++-
 src/security/security_selinux.c  | 20 ++-
 src/util/virhostdev.c| 53 
 11 files changed, 98 insertions(+), 114 deletions(-)

diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index d3f0449..370dc3a 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -390,6 +390,7 @@ virDomainAuditHostdev(virDomainObjPtr vm, 
virDomainHostdevDefPtr hostdev,
 const char *virt;
 virDomainHostdevSubsysUSBPtr usbsrc = hostdev-source.subsys.u.usb;
 virDomainHostdevSubsysPCIPtr pcisrc = hostdev-source.subsys.u.pci;
+virDomainHostdevSubsysSCSIPtr scsisrc = hostdev-source.subsys.u.scsi;
 
 virUUIDFormat(vm-def-uuid, uuidstr);
 if (!(vmname = virAuditEncode(vm, vm-def-name))) {
@@ -424,10 +425,8 @@ virDomainAuditHostdev(virDomainObjPtr vm, 
virDomainHostdevDefPtr hostdev,
 break;
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
 if (virAsprintfQuiet(address, %s:%d:%d:%d,
- hostdev-source.subsys.u.scsi.adapter,
- hostdev-source.subsys.u.scsi.bus,
- hostdev-source.subsys.u.scsi.target,
- hostdev-source.subsys.u.scsi.unit)  0) {
+ scsisrc-adapter, scsisrc-bus,
+ scsisrc-target, scsisrc-unit)  0) {
 VIR_WARN(OOM while encoding audit message);
 goto cleanup;
 }
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f1b9a46..e38771c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4025,6 +4025,7 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr node,
 bool got_address = false, got_adapter = false;
 xmlNodePtr cur;
 char *bus = NULL, *target = NULL, *unit = NULL;
+virDomainHostdevSubsysSCSIPtr scsisrc = def-source.subsys.u.scsi;
 
 cur = node-children;
 while (cur != NULL) {
@@ -4046,19 +4047,19 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr node,
 goto cleanup;
 }
 
-if (virStrToLong_ui(bus, NULL, 0, 
def-source.subsys.u.scsi.bus)  0) {
+if (virStrToLong_ui(bus, NULL, 0, scsisrc-bus)  0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(cannot parse bus '%s'), bus);
 goto cleanup;
 }
 
-if (virStrToLong_ui(target, NULL, 0, 
def-source.subsys.u.scsi.target)  0) {
+if (virStrToLong_ui(target, NULL, 0, scsisrc-target)  0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(cannot parse target '%s'), target);
 goto cleanup;
 }
 
-if (virStrToLong_ui(unit, NULL, 0, 
def-source.subsys.u.scsi.unit)  0) {
+if (virStrToLong_ui(unit, NULL, 0, scsisrc-unit)  0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(cannot parse unit '%s'), unit);
 goto cleanup;
@@ -4072,8 +4073,7 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr node,
  for scsi hostdev source));
 goto cleanup;
 }
-if (!(def-source.subsys.u.scsi.adapter =
-  virXMLPropString(cur, name))) {
+if (!(scsisrc-adapter = virXMLPropString(cur, name))) {
 virReportError(VIR_ERR_XML_ERROR, %s,
_('adapter' must be specified for scsi 
hostdev source));
 goto cleanup;
@@ -4254,6 +4254,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
 int backend;
 int ret = -1;
 virDomainHostdevSubsysPCIPtr pcisrc = def-source.subsys.u.pci;
+virDomainHostdevSubsysSCSIPtr scsisrc = def-source.subsys.u.scsi;
 
 /* @managed can be read from the xml document - it is always an
  * attribute of the toplevel element, no matter what type of
@@ -4311,8 +4312,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
 goto error;
 }
 
-if ((def-source.subsys.u.scsi.sgio =
- virDomainDeviceSGIOTypeFromString(sgio)) = 0) {
+if 

[libvirt] [PATCH 4/8] hostdev: Introduce virDomainHostdevSubsysSCSIHost

2014-07-11 Thread John Ferlan
Split virDomainHostdevSubsysSCSI further. In preparation for having
either SCSI or iSCSI data, create a union in virDomainHostdevSubsysSCSI
to contain just a virDomainHostdevSubsysSCSIHost to describe the
'scsi_host' host device

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/conf/domain_audit.c  |  8 +++---
 src/conf/domain_conf.c   | 58 
 src/conf/domain_conf.h   | 14 +++---
 src/qemu/qemu_cgroup.c   |  9 ---
 src/qemu/qemu_command.c  |  7 +++--
 src/qemu/qemu_conf.c | 18 +++--
 src/qemu/qemu_hotplug.c  | 13 +
 src/security/security_apparmor.c |  5 ++--
 src/security/security_dac.c  | 10 ---
 src/security/security_selinux.c  | 10 ---
 src/util/virhostdev.c| 36 -
 11 files changed, 113 insertions(+), 75 deletions(-)

diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index 370dc3a..ee9baa2 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -423,14 +423,16 @@ virDomainAuditHostdev(virDomainObjPtr vm, 
virDomainHostdevDefPtr hostdev,
 goto cleanup;
 }
 break;
-case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
+case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: {
+virDomainHostdevSubsysSCSIHostPtr scsihostsrc = scsisrc-u.host;
 if (virAsprintfQuiet(address, %s:%d:%d:%d,
- scsisrc-adapter, scsisrc-bus,
- scsisrc-target, scsisrc-unit)  0) {
+ scsihostsrc-adapter, scsihostsrc-bus,
+ scsihostsrc-target, scsihostsrc-unit)  0) {
 VIR_WARN(OOM while encoding audit message);
 goto cleanup;
 }
 break;
+}
 default:
 VIR_WARN(Unexpected hostdev type while encoding audit message: 
%d,
  hostdev-source.subsys.type);
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e38771c..55c0822 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1733,7 +1733,7 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def)
 break;
 case VIR_DOMAIN_HOSTDEV_MODE_SUBSYS:
 if (def-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)
-VIR_FREE(def-source.subsys.u.scsi.adapter);
+VIR_FREE(def-source.subsys.u.scsi.u.host.adapter);
 break;
 }
 }
@@ -4018,16 +4018,16 @@ virDomainHostdevSubsysPCIDefParseXML(xmlNodePtr node,
 }
 
 static int
-virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr node,
-  virDomainHostdevDefPtr def)
+virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr sourcenode,
+  virDomainHostdevSubsysSCSIPtr 
scsisrc)
 {
 int ret = -1;
 bool got_address = false, got_adapter = false;
 xmlNodePtr cur;
 char *bus = NULL, *target = NULL, *unit = NULL;
-virDomainHostdevSubsysSCSIPtr scsisrc = def-source.subsys.u.scsi;
+virDomainHostdevSubsysSCSIHostPtr scsihostsrc = scsisrc-u.host;
 
-cur = node-children;
+cur = sourcenode-children;
 while (cur != NULL) {
 if (cur-type == XML_ELEMENT_NODE) {
 if (xmlStrEqual(cur-name, BAD_CAST address)) {
@@ -4047,19 +4047,20 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr node,
 goto cleanup;
 }
 
-if (virStrToLong_ui(bus, NULL, 0, scsisrc-bus)  0) {
+if (virStrToLong_ui(bus, NULL, 0, scsihostsrc-bus)  0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(cannot parse bus '%s'), bus);
 goto cleanup;
 }
 
-if (virStrToLong_ui(target, NULL, 0, scsisrc-target)  0) {
+if (virStrToLong_ui(target, NULL, 0,
+scsihostsrc-target)  0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(cannot parse target '%s'), target);
 goto cleanup;
 }
 
-if (virStrToLong_ui(unit, NULL, 0, scsisrc-unit)  0) {
+if (virStrToLong_ui(unit, NULL, 0, scsihostsrc-unit)  0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(cannot parse unit '%s'), unit);
 goto cleanup;
@@ -4073,7 +4074,7 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr node,
  for scsi hostdev source));
 goto cleanup;
 }
-if (!(scsisrc-adapter = virXMLPropString(cur, name))) {
+if (!(scsihostsrc-adapter = virXMLPropString(cur, name))) {
 virReportError(VIR_ERR_XML_ERROR, %s,

[libvirt] [PATCH 0/8] Add iSCSI hostdev pass-through device

2014-07-11 Thread John Ferlan
https://bugzilla.redhat.com/show_bug.cgi?id=957293

Allow iSCSI hostdev/ devices to be added similarly to the existing
disk/ devices.  Initially do a little bit of refactoring of the
hostdev subsys structures to make it easier to model an iSCSI hostdev
after the SCSI scsi_host.  Although the bulk of the non-structure
changes are technically unnecessary, it just looked nicer to see
{usb|pci|scsi}src rather than the longer -source.subsys.u.{usb|pci|scsi}
in many places in the code.

The end result is the guest will have /dev/sdX devices created.

I have run this code through my Coverity environment and will be
running the virttest suite over the weekend.

Patches 1-3 are repetitive moves of the various hostdev subsys types
(USB, PCI, and SCSI) into separate typedefs and then modifying code use
the Ptr instead of the long union path to each field.  I think I got
most, but I'm sure there's still a few that could be cleaned up or
that were added since I started this.

Patch 4 more or less redoes patch 3 and I suppose could be combined. I
left it separate because it's showing the progression to patch 6. This
patch uses 'scsihost' to reference the specific portions while still
using 'scsisrc' to reference the shared portion (which is only sgio).

Patch 5 adds a virConnectPtr since patch 6 will need a way to get the
secret value for the iSCSI secret/auth in qemuBuildSCSIHostdevDrvStr.

Patch 6 adds the qemu code to handle a new hostdev protocol type
VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI. The code will mimic the
network disk (VIR_STORAGE_NET_PROTOCOL_ISCSI) code when making decisions.
The new 'scsisrc' field 'protocol' will be the decision point. The
default of VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE was chosen
over TYPE_SCSI_HOST. The changes were made in this order to reduce
the size of the patch when the XML is added (patch 8). Most of the
code was done inline using the iscsisrc-protocol (when the iSCSI code
had nothing to do). The remaining changes refactored code into SCSIHost 
and SCSIiSCSI to process the differences.

Patch 7 just refactors domain_conf to add what will be a common routine
to handle the host/ element network disks.  The iSCSI code will
reuse the code to have it's own element.

Patch 8 adds the XML, schema, and updates the docs to describe the
hostdev/ entry as well as of course adding new tests.  The tests
are a copy/merge of the scsi_host and network iscsi tests to define
and describe the expected model.

John Ferlan (8):
  hostdev: Introduce virDomainHostdevSubsysUSB
  hostdev: Introduce virDomainHostdevSubsysPCI
  hostdev: Introduce virDomainHostdevSubsysSCSI
  hostdev: Introduce virDomainHostdevSubsysSCSIHost
  Add virConnectPtr for qemuBuildSCSIHostdevDrvStr
  hostdev: Introduce virDomainHostdevSubsysSCSIiSCSI
  domain_conf: Common routine to handle network storage host xml def
  hostdev: Add iSCSI hostdev XML

 docs/formatdomain.html.in  | 142 +--
 docs/schemas/domaincommon.rng  |  46 +-
 src/conf/domain_audit.c|  38 +-
 src/conf/domain_conf.c | 463 +++--
 src/conf/domain_conf.h |  80 +++-
 src/libxl/libxl_conf.c |   9 +-
 src/libxl/libxl_domain.c   |   5 +-
 src/libxl/libxl_driver.c   |  34 +-
 src/lxc/lxc_cgroup.c   |   4 +-
 src/lxc/lxc_controller.c   |  10 +-
 src/lxc/lxc_driver.c   |  16 +-
 src/qemu/qemu_cgroup.c |  69 +--
 src/qemu/qemu_command.c| 158 ---
 src/qemu/qemu_command.h|   5 +-
 src/qemu/qemu_conf.c   |  20 +-
 src/qemu/qemu_driver.c |   6 +-
 src/qemu/qemu_hotplug.c|  93 +++--
 src/qemu/qemu_hotplug.h|   9 +-
 src/security/security_apparmor.c   |  33 +-
 src/security/security_dac.c|  66 +--
 src/security/security_selinux.c|  64 +--
 src/security/virt-aa-helper.c  |   5 +-
 src/util/virhostdev.c  | 295 +++--
 .../qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args  |  14 +
 .../qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.xml   |  46 ++
 .../qemuxml2argv-hostdev-scsi-lsi-iscsi.args   |  14 +
 .../qemuxml2argv-hostdev-scsi-lsi-iscsi.xml|  40 ++
 ...emuxml2argv-hostdev-scsi-virtio-iscsi-auth.args |  16 +
 ...qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.xml |  46 ++
 .../qemuxml2argv-hostdev-scsi-virtio-iscsi.args|  16 +
 .../qemuxml2argv-hostdev-scsi-virtio-iscsi.xml |  40 ++
 tests/qemuxml2argvtest.c   |  16 +
 tests/qemuxml2xmltest.c|   5 +
 33 files changed, 1317 insertions(+), 606 

[libvirt] [PATCH 2/8] hostdev: Introduce virDomainHostdevSubsysPCI

2014-07-11 Thread John Ferlan
Create a separate typedef for the hostdev union data describing PCI.
Then adjust the code to use the new pointer

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/conf/domain_audit.c  |  9 
 src/conf/domain_conf.c   | 25 -
 src/conf/domain_conf.h   | 12 ++
 src/libxl/libxl_conf.c   |  9 
 src/libxl/libxl_domain.c |  5 +++--
 src/libxl/libxl_driver.c | 34 ++---
 src/qemu/qemu_cgroup.c   | 24 ++--
 src/qemu/qemu_command.c  | 47 ++--
 src/qemu/qemu_hotplug.c  | 11 +-
 src/security/security_apparmor.c | 10 -
 src/security/security_dac.c  | 20 +++--
 src/security/security_selinux.c  | 20 +++--
 src/util/virhostdev.c| 34 -
 13 files changed, 125 insertions(+), 135 deletions(-)

diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index 8277b06..d3f0449 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -389,6 +389,7 @@ virDomainAuditHostdev(virDomainObjPtr vm, 
virDomainHostdevDefPtr hostdev,
 char *device = NULL;
 const char *virt;
 virDomainHostdevSubsysUSBPtr usbsrc = hostdev-source.subsys.u.usb;
+virDomainHostdevSubsysPCIPtr pcisrc = hostdev-source.subsys.u.pci;
 
 virUUIDFormat(vm-def-uuid, uuidstr);
 if (!(vmname = virAuditEncode(vm, vm-def-name))) {
@@ -406,10 +407,10 @@ virDomainAuditHostdev(virDomainObjPtr vm, 
virDomainHostdevDefPtr hostdev,
 switch (hostdev-source.subsys.type) {
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
 if (virAsprintfQuiet(address, %.4x:%.2x:%.2x.%.1x,
- hostdev-source.subsys.u.pci.addr.domain,
- hostdev-source.subsys.u.pci.addr.bus,
- hostdev-source.subsys.u.pci.addr.slot,
- hostdev-source.subsys.u.pci.addr.function)  
0) {
+ pcisrc-addr.domain,
+ pcisrc-addr.bus,
+ pcisrc-addr.slot,
+ pcisrc-addr.function)  0) {
 VIR_WARN(OOM while encoding audit message);
 goto cleanup;
 }
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 046b4f8..f1b9a46 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4253,6 +4253,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
 char *backendStr = NULL;
 int backend;
 int ret = -1;
+virDomainHostdevSubsysPCIPtr pcisrc = def-source.subsys.u.pci;
 
 /* @managed can be read from the xml document - it is always an
  * attribute of the toplevel element, no matter what type of
@@ -4332,7 +4333,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
  has been specified), backendStr);
 goto error;
 }
-def-source.subsys.u.pci.backend = backend;
+pcisrc-backend = backend;
 
 break;
 
@@ -10123,10 +10124,13 @@ static int
 virDomainHostdevMatchSubsysPCI(virDomainHostdevDefPtr a,
virDomainHostdevDefPtr b)
 {
-if (a-source.subsys.u.pci.addr.domain == 
b-source.subsys.u.pci.addr.domain 
-a-source.subsys.u.pci.addr.bus == b-source.subsys.u.pci.addr.bus 
-a-source.subsys.u.pci.addr.slot == b-source.subsys.u.pci.addr.slot 
-a-source.subsys.u.pci.addr.function == 
b-source.subsys.u.pci.addr.function)
+virDomainHostdevSubsysPCIPtr apcisrc = a-source.subsys.u.pci;
+virDomainHostdevSubsysPCIPtr bpcisrc = b-source.subsys.u.pci;
+
+if (apcisrc-addr.domain == bpcisrc-addr.domain 
+apcisrc-addr.bus == bpcisrc-addr.bus 
+apcisrc-addr.slot == bpcisrc-addr.slot 
+apcisrc-addr.function == bpcisrc-addr.function)
 return 1;
 return 0;
 }
@@ -15486,15 +15490,17 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf,
 bool includeTypeInAddr)
 {
 virDomainHostdevSubsysUSBPtr usbsrc = def-source.subsys.u.usb;
+virDomainHostdevSubsysPCIPtr pcisrc = def-source.subsys.u.pci;
 
 if (def-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI 
-def-source.subsys.u.pci.backend != 
VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) {
-const char *backend = 
virDomainHostdevSubsysPCIBackendTypeToString(def-source.subsys.u.pci.backend);
+pcisrc-backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) {
+const char *backend =
+virDomainHostdevSubsysPCIBackendTypeToString(pcisrc-backend);
 
 if (!backend) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(unexpected pci hostdev driver name type %d),
-   def-source.subsys.u.pci.backend);
+   

[libvirt] [PATCH 5/8] Add virConnectPtr for qemuBuildSCSIHostdevDrvStr

2014-07-11 Thread John Ferlan
Add a conn for future patches to be able to grab the secret when
authenticating an iSCSI host device

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/qemu/qemu_command.c |  5 +++--
 src/qemu/qemu_command.h |  5 +++--
 src/qemu/qemu_driver.c  |  6 +++---
 src/qemu/qemu_hotplug.c | 34 --
 src/qemu/qemu_hotplug.h |  9 ++---
 5 files changed, 35 insertions(+), 24 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index dbdb871..1ab3ade 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5120,7 +5120,8 @@ qemuBuildUSBHostdevUSBDevStr(virDomainHostdevDefPtr dev)
 }
 
 char *
-qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
+qemuBuildSCSIHostdevDrvStr(virConnectPtr conn ATTRIBUTE_UNUSED,
+   virDomainHostdevDefPtr dev,
virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED,
qemuBuildCommandLineCallbacksPtr callbacks)
 {
@@ -8866,7 +8867,7 @@ qemuBuildCommandLine(virConnectPtr conn,
 char *drvstr;
 
 virCommandAddArg(cmd, -drive);
-if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev, qemuCaps, 
callbacks)))
+if (!(drvstr = qemuBuildSCSIHostdevDrvStr(conn, hostdev, 
qemuCaps, callbacks)))
 goto error;
 virCommandAddArg(cmd, drvstr);
 VIR_FREE(drvstr);
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index cf51056..b71e964 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -170,10 +170,11 @@ char * qemuBuildUSBHostdevDevStr(virDomainDefPtr def,
  virDomainHostdevDefPtr dev,
  virQEMUCapsPtr qemuCaps);
 
-char * qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
+char * qemuBuildSCSIHostdevDrvStr(virConnectPtr conn,
+  virDomainHostdevDefPtr dev,
   virQEMUCapsPtr qemuCaps,
   qemuBuildCommandLineCallbacksPtr callbacks)
-ATTRIBUTE_NONNULL(3);
+ATTRIBUTE_NONNULL(4);
 char * qemuBuildSCSIHostdevDevStr(virDomainDefPtr def,
   virDomainHostdevDefPtr dev,
   virQEMUCapsPtr qemuCaps);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ecccf6c..3c6fef7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6472,7 +6472,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
 break;
 
 case VIR_DOMAIN_DEVICE_HOSTDEV:
-ret = qemuDomainAttachHostDevice(driver, vm,
+ret = qemuDomainAttachHostDevice(dom-conn, driver, vm,
  dev-data.hostdev);
 if (!ret)
 dev-data.hostdev = NULL;
@@ -6544,10 +6544,10 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
 ret = qemuDomainDetachLease(driver, vm, dev-data.lease);
 break;
 case VIR_DOMAIN_DEVICE_NET:
-ret = qemuDomainDetachNetDevice(driver, vm, dev);
+ret = qemuDomainDetachNetDevice(dom-conn, driver, vm, dev);
 break;
 case VIR_DOMAIN_DEVICE_HOSTDEV:
-ret = qemuDomainDetachHostDevice(driver, vm, dev);
+ret = qemuDomainDetachHostDevice(dom-conn, driver, vm, dev);
 break;
 case VIR_DOMAIN_DEVICE_CHR:
 ret = qemuDomainDetachChrDevice(driver, vm, dev-data.chr);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 7df5832..6773d50 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -860,7 +860,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
  * netdev-specific code as appropriate), then also added to
  * the nets list (see cleanup:) if successful.
  */
-ret = qemuDomainAttachHostDevice(driver, vm,
+ret = qemuDomainAttachHostDevice(conn, driver, vm,
  virDomainNetGetActualHostdev(net));
 goto cleanup;
 }
@@ -1547,7 +1547,8 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
 }
 
 static int
-qemuDomainAttachHostSCSIDevice(virQEMUDriverPtr driver,
+qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
+   virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainHostdevDefPtr hostdev)
 {
@@ -1594,7 +1595,7 @@ qemuDomainAttachHostSCSIDevice(virQEMUDriverPtr driver,
 if (qemuAssignDeviceHostdevAlias(vm-def, hostdev, -1)  0)
 goto cleanup;
 
-if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev, priv-qemuCaps,
+if (!(drvstr = qemuBuildSCSIHostdevDrvStr(conn, hostdev, priv-qemuCaps,
   buildCommandLineCallbacks)))
 goto cleanup;
 
@@ -1642,7 +1643,8 @@ qemuDomainAttachHostSCSIDevice(virQEMUDriverPtr driver,
 return ret;
 }
 
-int qemuDomainAttachHostDevice(virQEMUDriverPtr 

[libvirt] [PATCH 8/8] hostdev: Add iSCSI hostdev XML

2014-07-11 Thread John Ferlan
Introduce a new structure to handle an iSCSI host device based on the
existing virDomainHostdevSubsysSCSI by adding a protocol='iscsi' to
the source/ element.  The hostdev structure mimics the existing
disk/ element for an iSCSI device (network) device. New XML is:

  hostdev mode='subsystem' type='scsi' managed='yes'
auth username='myname'
  secret type='iscsi' usage='mycluster_myname'/
/auth
source protocol='iscsi' name='iqn.1992-01.com.example'
  host name='example.org' port='3260'/
/source
address type='drive' controller='0' bus='0' target='2' unit='5'/
  /hostdev

The controller element will mimic the existing scsi_host code insomuch
as when 'lsi' and 'virtio-scsi' are used.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 docs/formatdomain.html.in  | 142 ---
 docs/schemas/domaincommon.rng  |  46 ++-
 src/conf/domain_conf.c | 152 ++---
 .../qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args  |  14 ++
 .../qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.xml   |  46 +++
 .../qemuxml2argv-hostdev-scsi-lsi-iscsi.args   |  14 ++
 .../qemuxml2argv-hostdev-scsi-lsi-iscsi.xml|  40 ++
 ...emuxml2argv-hostdev-scsi-virtio-iscsi-auth.args |  16 +++
 ...qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.xml |  46 +++
 .../qemuxml2argv-hostdev-scsi-virtio-iscsi.args|  16 +++
 .../qemuxml2argv-hostdev-scsi-virtio-iscsi.xml |  40 ++
 tests/qemuxml2argvtest.c   |  16 +++
 tests/qemuxml2xmltest.c|   5 +
 13 files changed, 524 insertions(+), 69 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-lsi-iscsi.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-virtio-iscsi.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index b69da4c..82b9091 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2781,57 +2781,107 @@
   lt;/devicesgt;
   .../pre
 
+
+por:/p
+
+pre
+  ...
+  lt;devicesgt;
+lt;hostdev mode='subsystem' type='scsi'gt;
+  lt;source protocol='iscsi' 
name='iqn.2014-08.com.example:iscsi-nopool/1'gt;
+lt;host name='example.com' port='3260'/gt;
+  lt;/sourcegt;
+  lt;auth username='myuser'gt;
+lt;secret type='iscsi' usage='libvirtiscsi'/gt;
+  lt;/authgt;
+  lt;address type='drive' controller='0' bus='0' target='0' unit='0'/gt;
+lt;/hostdevgt;
+  lt;/devicesgt;
+  .../pre
+
 dl
   dtcodehostdev/code/dt
   ddThe codehostdev/code element is the main container for describing
-host devices. For usb device passthrough codemode/code is always
-subsystem and codetype/code is usb for a USB device, pci
-for a PCI device and scsi for a SCSI device. When
-codemanaged/code is yes for a PCI
-device, it is detached from the host before being passed on to
-the guest, and reattached to the host after the guest exits.
-If codemanaged/code is omitted or no, and for USB
-devices, the user is responsible to
-call codevirNodeDeviceDettach/code (or codevirsh
-nodedev-dettach/code) before starting the guest or
-hot-plugging the device,
-and codevirNodeDeviceReAttach/code (or codevirsh
-nodedev-reattach/code) after hot-unplug or stopping the
-guest. For SCSI device, user is responsible to make sure the device
-is not used by host.
-The optional codesgio/code (span class=sincesince 1.0.6/span)
-attribute indicates whether the kernel will filter unprivileged
-SG_IO commands for the disk, valid settings are filtered or
-unfiltered. Defaults to filtered.
+host devices. For each device, the codemode/code is always
+subsystem and the codetype/code is one of the following values
+with additional attributes noted.
+dl
+  dtusb/dt
+  ddFor USB devices, the user is responsible to call
+codevirNodeDeviceDettach/code (or
+codevirsh nodedev-dettach/code) before starting the guest
+or hot-plugging the device and codevirNodeDeviceReAttach/code
+(or codevirsh nodedev-reattach/code) after hot-unplug or
+stopping the guest.
+  /dd
+  dtpci/dt
+  ddFor PCI devices, when codemanaged/code is yes 

[libvirt] [PATCH 6/8] hostdev: Introduce virDomainHostdevSubsysSCSIiSCSI

2014-07-11 Thread John Ferlan
Create the structures and API's to hold and manage the iSCSI host device.
This extends the 'scsi_host' definitions added in commit id '5c811dce'.
A future patch will add the XML parsing, but that code requires some
infrastructure to be in place first in order to handle the differences
between a 'scsi_host' and an 'iSCSI host' device.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/conf/domain_audit.c  |  20 +++-
 src/conf/domain_conf.c   |  47 -
 src/conf/domain_conf.h   |  20 
 src/qemu/qemu_cgroup.c   |  35 ---
 src/qemu/qemu_command.c  |  74 ---
 src/qemu/qemu_hotplug.c  |  36 +--
 src/security/security_apparmor.c |   6 ++
 src/security/security_dac.c  |  12 +++
 src/security/security_selinux.c  |  12 +++
 src/util/virhostdev.c| 200 +--
 10 files changed, 349 insertions(+), 113 deletions(-)

diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index ee9baa2..3ad58b0 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -424,12 +424,22 @@ virDomainAuditHostdev(virDomainObjPtr vm, 
virDomainHostdevDefPtr hostdev,
 }
 break;
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: {
-virDomainHostdevSubsysSCSIHostPtr scsihostsrc = scsisrc-u.host;
-if (virAsprintfQuiet(address, %s:%d:%d:%d,
- scsihostsrc-adapter, scsihostsrc-bus,
- scsihostsrc-target, scsihostsrc-unit)  0) {
-VIR_WARN(OOM while encoding audit message);
+if (scsisrc-protocol ==
+VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) {
+/* Follow virDomainAuditDisk  virDomainAuditGenericDev
+ * and don't audit the networked device.
+ */
 goto cleanup;
+} else {
+virDomainHostdevSubsysSCSIHostPtr scsihostsrc =
+scsisrc-u.host;
+if (virAsprintfQuiet(address, %s:%d:%d:%d,
+ scsihostsrc-adapter, scsihostsrc-bus,
+ scsihostsrc-target,
+ scsihostsrc-unit)  0) {
+VIR_WARN(OOM while encoding audit message);
+goto cleanup;
+}
 }
 break;
 }
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 55c0822..758b907 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1702,6 +1702,17 @@ virDomainHostdevDefPtr virDomainHostdevDefAlloc(void)
 return def;
 }
 
+static void
+virDomainHostdevSubsysSCSIiSCSIFree(virDomainHostdevSubsysSCSIiSCSIPtr 
iscsisrc)
+{
+if (!iscsisrc)
+return;
+VIR_FREE(iscsisrc-path);
+virStorageNetHostDefFree(iscsisrc-nhosts, iscsisrc-hosts);
+virStorageAuthDefFree(iscsisrc-auth);
+iscsisrc-auth = NULL;
+}
+
 void virDomainHostdevDefClear(virDomainHostdevDefPtr def)
 {
 if (!def)
@@ -1732,8 +1743,15 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def)
 }
 break;
 case VIR_DOMAIN_HOSTDEV_MODE_SUBSYS:
-if (def-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI)
-VIR_FREE(def-source.subsys.u.scsi.u.host.adapter);
+if (def-source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) {
+virDomainHostdevSubsysSCSIPtr scsisrc = def-source.subsys.u.scsi;
+if (scsisrc-protocol ==
+VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) {
+virDomainHostdevSubsysSCSIiSCSIFree(scsisrc-u.iscsi);
+} else {
+VIR_FREE(scsisrc-u.host.adapter);
+}
+}
 break;
 }
 }
@@ -4313,8 +4331,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
 }
 
 if (sgio) {
-if (def-source.subsys.type !=
-VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) {
+if (def-source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) {
 virReportError(VIR_ERR_XML_ERROR, %s,
_(sgio is only supported for scsi host device));
 goto error;
@@ -10161,6 +10178,22 @@ 
virDomainHostdevMatchSubsysSCSIHost(virDomainHostdevDefPtr a,
 }
 
 static int
+virDomainHostdevMatchSubsysSCSIiSCSI(virDomainHostdevDefPtr a,
+ virDomainHostdevDefPtr b)
+{
+virDomainHostdevSubsysSCSIiSCSIPtr aiscsisrc =
+a-source.subsys.u.scsi.u.iscsi;
+virDomainHostdevSubsysSCSIiSCSIPtr biscsisrc =
+b-source.subsys.u.scsi.u.iscsi;
+
+if (STREQ(aiscsisrc-hosts[0].name, biscsisrc-hosts[0].name) 
+STREQ(aiscsisrc-hosts[0].port, biscsisrc-hosts[0].port) 
+STREQ(aiscsisrc-path, biscsisrc-path))
+return 1;
+return 0;
+}
+
+static int
 virDomainHostdevMatchSubsys(virDomainHostdevDefPtr a,
 

[libvirt] [PATCH 7/8] domain_conf: Common routine to handle network storage host xml def

2014-07-11 Thread John Ferlan
In preparation for hostdev support for iSCSI and a virStorageNetHostDefPtr,
split out the network disk storage parsing of the 'host' element into a
separate routine.

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/conf/domain_conf.c | 137 +++--
 1 file changed, 76 insertions(+), 61 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 758b907..814ab87 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4036,6 +4036,79 @@ virDomainHostdevSubsysPCIDefParseXML(xmlNodePtr node,
 }
 
 static int
+virDomainStorageHostParse(xmlNodePtr node,
+  virStorageNetHostDefPtr *hosts,
+  size_t *nhosts)
+{
+int ret = -1;
+xmlNodePtr child;
+char *transport = NULL;
+virStorageNetHostDef host;
+
+memset(host, 0, sizeof(host));
+
+child = node-children;
+while (child != NULL) {
+if (child-type == XML_ELEMENT_NODE 
+xmlStrEqual(child-name, BAD_CAST host)) {
+
+host.transport = VIR_STORAGE_NET_HOST_TRANS_TCP;
+
+/* transport can be tcp (default), unix or rdma.  */
+if ((transport = virXMLPropString(child, transport))) {
+host.transport = 
virStorageNetHostTransportTypeFromString(transport);
+if (host.transport  0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(unknown protocol transport type '%s'),
+   transport);
+goto cleanup;
+}
+}
+
+host.socket = virXMLPropString(child, socket);
+
+if (host.transport == VIR_STORAGE_NET_HOST_TRANS_UNIX 
+host.socket == NULL) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(missing socket for unix transport));
+goto cleanup;
+}
+
+if (host.transport != VIR_STORAGE_NET_HOST_TRANS_UNIX 
+host.socket != NULL) {
+virReportError(VIR_ERR_XML_ERROR,
+   _(transport '%s' does not support 
+ socket attribute),
+   transport);
+goto cleanup;
+}
+
+VIR_FREE(transport);
+
+if (host.transport != VIR_STORAGE_NET_HOST_TRANS_UNIX) {
+if (!(host.name = virXMLPropString(child, name))) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(missing name for host));
+goto cleanup;
+}
+
+host.port = virXMLPropString(child, port);
+}
+
+if (VIR_APPEND_ELEMENT(*hosts, *nhosts, host)  0)
+goto cleanup;
+}
+child = child-next;
+}
+ret = 0;
+
+ cleanup:
+virStorageNetHostDefClear(host);
+VIR_FREE(transport);
+return ret;
+}
+
+static int
 virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr sourcenode,
   virDomainHostdevSubsysSCSIPtr 
scsisrc)
 {
@@ -5014,13 +5087,8 @@ int
 virDomainDiskSourceParse(xmlNodePtr node,
  virStorageSourcePtr src)
 {
-char *protocol = NULL;
-char *transport = NULL;
-virStorageNetHostDef host;
-xmlNodePtr child;
 int ret = -1;
-
-memset(host, 0, sizeof(host));
+char *protocol = NULL;
 
 switch ((virStorageType)src-type) {
 case VIR_STORAGE_TYPE_FILE:
@@ -5073,59 +5141,8 @@ virDomainDiskSourceParse(xmlNodePtr node,
 tmp[0] = '\0';
 }
 
-child = node-children;
-while (child != NULL) {
-if (child-type == XML_ELEMENT_NODE 
-xmlStrEqual(child-name, BAD_CAST host)) {
-
-host.transport = VIR_STORAGE_NET_HOST_TRANS_TCP;
-
-/* transport can be tcp (default), unix or rdma.  */
-if ((transport = virXMLPropString(child, transport))) {
-host.transport = 
virStorageNetHostTransportTypeFromString(transport);
-if (host.transport  0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _(unknown protocol transport type 
'%s'),
-   transport);
-goto cleanup;
-}
-}
-
-host.socket = virXMLPropString(child, socket);
-
-if (host.transport == VIR_STORAGE_NET_HOST_TRANS_UNIX 
-host.socket == NULL) {
-virReportError(VIR_ERR_XML_ERROR, %s,
-   _(missing socket for unix transport));
-goto cleanup;
-}
-
-if (host.transport != VIR_STORAGE_NET_HOST_TRANS_UNIX 
-host.socket != NULL) {
-   

[libvirt] [PATCH 1/8] hostdev: Introduce virDomainHostdevSubsysUSB

2014-07-11 Thread John Ferlan
Create a separate typedef for the hostdev union data describing USB.
Then adjust the code to use the new pointer

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/conf/domain_audit.c  |  4 ++--
 src/conf/domain_conf.c   | 50 +++-
 src/conf/domain_conf.h   | 22 ++
 src/lxc/lxc_cgroup.c |  4 ++--
 src/lxc/lxc_controller.c | 10 
 src/lxc/lxc_driver.c | 16 ++---
 src/qemu/qemu_cgroup.c   |  4 ++--
 src/qemu/qemu_command.c  | 26 ++---
 src/qemu/qemu_hotplug.c  |  7 +++---
 src/security/security_apparmor.c |  6 ++---
 src/security/security_dac.c  | 12 --
 src/security/security_selinux.c  | 10 
 src/security/virt-aa-helper.c|  5 ++--
 src/util/virhostdev.c| 50 +++-
 14 files changed, 104 insertions(+), 122 deletions(-)

diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index a3d6c67..8277b06 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -388,6 +388,7 @@ virDomainAuditHostdev(virDomainObjPtr vm, 
virDomainHostdevDefPtr hostdev,
 char *address = NULL;
 char *device = NULL;
 const char *virt;
+virDomainHostdevSubsysUSBPtr usbsrc = hostdev-source.subsys.u.usb;
 
 virUUIDFormat(vm-def-uuid, uuidstr);
 if (!(vmname = virAuditEncode(vm, vm-def-name))) {
@@ -415,8 +416,7 @@ virDomainAuditHostdev(virDomainObjPtr vm, 
virDomainHostdevDefPtr hostdev,
 break;
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
 if (virAsprintfQuiet(address, %.3d.%.3d,
- hostdev-source.subsys.u.usb.bus,
- hostdev-source.subsys.u.usb.device)  0) {
+ usbsrc-bus, usbsrc-device)  0) {
 VIR_WARN(OOM while encoding audit message);
 goto cleanup;
 }
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b91ccf7..046b4f8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3800,6 +3800,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
 xmlNodePtr cur;
 char *startupPolicy = NULL;
 char *autoAddress;
+virDomainHostdevSubsysUSBPtr usbsrc = def-source.subsys.u.usb;
 
 if ((startupPolicy = virXMLPropString(node, startupPolicy))) {
 def-startupPolicy =
@@ -3816,7 +3817,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
 
 if ((autoAddress = virXMLPropString(node, autoAddress))) {
 if (STREQ(autoAddress, yes))
-def-source.subsys.u.usb.autoAddress = true;
+usbsrc-autoAddress = true;
 VIR_FREE(autoAddress);
 }
 
@@ -3833,8 +3834,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
 
 if (vendor) {
 got_vendor = true;
-if (virStrToLong_ui(vendor, NULL, 0,
-def-source.subsys.u.usb.vendor)  0) {
+if (virStrToLong_ui(vendor, NULL, 0, usbsrc-vendor)  0) 
{
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(cannot parse vendor id %s), vendor);
 VIR_FREE(vendor);
@@ -3852,7 +3852,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
 if (product) {
 got_product = true;
 if (virStrToLong_ui(product, NULL, 0,
-def-source.subsys.u.usb.product)  
0) {
+usbsrc-product)  0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(cannot parse product %s),
product);
@@ -3870,8 +3870,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
 
 bus = virXMLPropString(cur, bus);
 if (bus) {
-if (virStrToLong_ui(bus, NULL, 0,
-def-source.subsys.u.usb.bus)  0) {
+if (virStrToLong_ui(bus, NULL, 0, usbsrc-bus)  0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(cannot parse bus %s), bus);
 VIR_FREE(bus);
@@ -3886,8 +3885,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
 
 device = virXMLPropString(cur, device);
 if (device) {
-if (virStrToLong_ui(device, NULL, 0,
-def-source.subsys.u.usb.device)  0) 
 {
+if (virStrToLong_ui(device, NULL, 0, usbsrc-device)  0) 
{
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(cannot parse device %s),
device);
@@ -3910,7 +3908,7 @@ 

[libvirt] [PATCHv2 1/2] Don't output libvirt-UUID.files for LXC apparmor profiles

2014-07-11 Thread Cédric Bosdonnat
---
 src/security/virt-aa-helper.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index b5f66f3..c8f17f9 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1342,10 +1342,13 @@ main(int argc, char **argv)
 vah_info(include_file);
 vah_info(included_files);
 rc = 0;
+} else if (ctl-def-virtType == VIR_DOMAIN_VIRT_LXC) {
+rc = 0;
 } else if ((rc = update_include_file(include_file,
  included_files,
- ctl-append)) != 0)
+ ctl-append)) != 0) {
 goto cleanup;
+}
 
 
 /* create the profile from TEMPLATE */
-- 
1.8.4.5

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


[libvirt] [PATCHv2 0/2] AppArmor lxc profile fixes

2014-07-11 Thread Cédric Bosdonnat
Diff to v1:
  * virt-aa-helper: don't return -1 when reloading the profile: nothing
to do isn't a bad thing sometimes.

Cédric Bosdonnat (2):
  Don't output libvirt-UUID.files for LXC apparmor profiles
  Rework lxc apparmor profile

 examples/apparmor/Makefile.am |   6 +-
 examples/apparmor/TEMPLATE.lxc|  15 
 examples/apparmor/{TEMPLATE = TEMPLATE.qemu} |   2 +-
 examples/apparmor/libvirt-lxc | 119 +++---
 src/security/security_apparmor.c  |  20 +++--
 src/security/virt-aa-helper.c |  34 ++--
 6 files changed, 152 insertions(+), 44 deletions(-)
 create mode 100644 examples/apparmor/TEMPLATE.lxc
 rename examples/apparmor/{TEMPLATE = TEMPLATE.qemu} (75%)

-- 
1.8.4.5

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

[libvirt] [PATCHv2 2/2] Rework lxc apparmor profile

2014-07-11 Thread Cédric Bosdonnat
Rework the apparmor lxc profile abstraction to mimic ubuntu's container-default.
This profile allows quite a lot, but strives to restrict access to
dangerous resources.

Removing the explicit authorizations to bash, systemd and cron files,
forces them to keep the lxc profile for all applications inside the
container. PUx permissions where leading to running systemd (and others
tasks) unconfined.

Put the generic files, network and capabilities restrictions directly
in the TEMPLATE.lxc: this way, users can restrict them on a per
container basis.
---
 examples/apparmor/Makefile.am |   6 +-
 examples/apparmor/TEMPLATE.lxc|  15 
 examples/apparmor/{TEMPLATE = TEMPLATE.qemu} |   2 +-
 examples/apparmor/libvirt-lxc | 119 +++---
 src/security/security_apparmor.c  |  20 +++--
 src/security/virt-aa-helper.c |  29 +--
 6 files changed, 148 insertions(+), 43 deletions(-)
 create mode 100644 examples/apparmor/TEMPLATE.lxc
 rename examples/apparmor/{TEMPLATE = TEMPLATE.qemu} (75%)

diff --git a/examples/apparmor/Makefile.am b/examples/apparmor/Makefile.am
index a741e94..7a20e16 100644
--- a/examples/apparmor/Makefile.am
+++ b/examples/apparmor/Makefile.am
@@ -15,7 +15,8 @@
 ## http://www.gnu.org/licenses/.
 
 EXTRA_DIST=\
-   TEMPLATE\
+   TEMPLATE.qemu   \
+   TEMPLATE.lxc\
libvirt-qemu\
libvirt-lxc \
usr.lib.libvirt.virt-aa-helper  \
@@ -36,6 +37,7 @@ abstractions_DATA = \
 
 templatesdir = $(apparmordir)/libvirt
 templates_DATA = \
-   TEMPLATE \
+   TEMPLATE.qemu \
+   TEMPLATE.lxc \
$(NULL)
 endif WITH_APPARMOR_PROFILES
diff --git a/examples/apparmor/TEMPLATE.lxc b/examples/apparmor/TEMPLATE.lxc
new file mode 100644
index 000..7b64885
--- /dev/null
+++ b/examples/apparmor/TEMPLATE.lxc
@@ -0,0 +1,15 @@
+#
+# This profile is for the domain whose UUID matches this file.
+#
+
+#include tunables/global
+
+profile LIBVIRT_TEMPLATE {
+  #include abstractions/libvirt-lxc
+
+  # Globally allows everything to run under this profile
+  # These can be narrowed depending on the container's use.
+  file,
+  capability,
+  network,
+}
diff --git a/examples/apparmor/TEMPLATE b/examples/apparmor/TEMPLATE.qemu
similarity index 75%
rename from examples/apparmor/TEMPLATE
rename to examples/apparmor/TEMPLATE.qemu
index 187dec5..008a221 100644
--- a/examples/apparmor/TEMPLATE
+++ b/examples/apparmor/TEMPLATE.qemu
@@ -5,5 +5,5 @@
 #include tunables/global
 
 profile LIBVIRT_TEMPLATE {
-  #include abstractions/libvirt-driver
+  #include abstractions/libvirt-qemu
 }
diff --git a/examples/apparmor/libvirt-lxc b/examples/apparmor/libvirt-lxc
index d404328..4bfb503 100644
--- a/examples/apparmor/libvirt-lxc
+++ b/examples/apparmor/libvirt-lxc
@@ -2,16 +2,115 @@
 
   #include abstractions/base
 
-  # Needed for lxc-enter-namespace
-  capability sys_admin,
-  capability sys_chroot,
+  umount,
 
-  # Added for lxc-enter-namespace --cmd /bin/bash
-  /bin/bash PUx,
+  # ignore DENIED message on / remount
+  deny mount options=(ro, remount) - /,
 
-  /usr/sbin/cron PUx,
-  /usr/lib/systemd/systemd PUx,
+  # allow tmpfs mounts everywhere
+  mount fstype=tmpfs,
 
-  /usr/lib/libsystemd-*.so.* mr,
-  /usr/lib/libudev-*.so.* mr,
-  /etc/ld.so.cache mr,
+  # allow mqueue mounts everywhere
+  mount fstype=mqueue,
+
+  # allow fuse mounts everywhere
+  mount fstype=fuse.*,
+
+  # deny writes in /proc/sys/fs but allow binfmt_misc to be mounted
+  mount fstype=binfmt_misc - /proc/sys/fs/binfmt_misc/,
+  deny @{PROC}/sys/fs/** wklx,
+
+  # allow efivars to be mounted, writing to it will be blocked though
+  mount fstype=efivarfs - /sys/firmware/efi/efivars/,
+
+  # block some other dangerous paths
+  deny @{PROC}/sysrq-trigger rwklx,
+  deny @{PROC}/mem rwklx,
+  deny @{PROC}/kmem rwklx,
+
+  # deny writes in /sys except for /sys/fs/cgroup, also allow
+  # fusectl, securityfs and debugfs to be mounted there (read-only)
+  mount fstype=fusectl - /sys/fs/fuse/connections/,
+  mount fstype=securityfs - /sys/kernel/security/,
+  mount fstype=debugfs - /sys/kernel/debug/,
+  mount fstype=proc - /proc/,
+  mount fstype=sysfs - /sys/,
+  deny /sys/firmware/efi/efivars/** rwklx,
+  deny /sys/kernel/security/** rwklx,
+
+  # generated by: lxc-generate-aa-rules.py container-rules.base
+  deny /proc/sys/[^kn]*{,/**} wklx,
+  deny /proc/sys/k[^e]*{,/**} wklx,
+  deny /proc/sys/ke[^r]*{,/**} wklx,
+  deny /proc/sys/ker[^n]*{,/**} wklx,
+  deny /proc/sys/kern[^e]*{,/**} wklx,
+  deny /proc/sys/kerne[^l]*{,/**} wklx,
+  deny /proc/sys/kernel/[^smhd]*{,/**} wklx,
+  deny /proc/sys/kernel/d[^o]*{,/**} wklx,
+  deny /proc/sys/kernel/do[^m]*{,/**} wklx,
+  deny /proc/sys/kernel/dom[^a]*{,/**} wklx,
+  deny /proc/sys/kernel/doma[^i]*{,/**} wklx,
+  deny /proc/sys/kernel/domai[^n]*{,/**} 

[libvirt] [PATCH] virseclabel: Resolve Coverity FORWARD_NULL issue

2014-07-11 Thread John Ferlan
Resolve issue introduced by commit id '13adf1b'

Signed-off-by: John Ferlan jfer...@redhat.com
---
 src/util/virseclabel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/virseclabel.c b/src/util/virseclabel.c
index e9d973c..02a8342 100644
--- a/src/util/virseclabel.c
+++ b/src/util/virseclabel.c
@@ -61,7 +61,7 @@ virSecurityLabelDefNew(const char *model)
 if (VIR_ALLOC(seclabel)  0 ||
 VIR_STRDUP(seclabel-model, model)  0) {
 virSecurityLabelDefFree(seclabel);
-seclabel = NULL;
+return NULL;
 }
 
 seclabel-relabel = true;
-- 
1.9.3

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


[libvirt] [PATCH] libxl: Correct cast to virDomainDiskDiscard enum.

2014-07-11 Thread Jim Fehlig
From: Ian Campbell ian.campb...@citrix.com

This was converted to a typedef in 5a2bd4c9171d conf: more enum
cleanups in src/conf/domain_conf.h causing:
libxl/libxl_conf.c: In function 'libxlDiskSetDiscard':
libxl/libxl_conf.c:724:19: error: conversion to incomplete type

Signed-off-by: Ian Campbell ian.campb...@citrix.com
---

This build failure was found by the osstest build bot

http://lists.xen.org/archives/html/xen-devel/2014-07/msg01552.html

Pushing under the build-breaker rule.

 src/libxl/libxl_conf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 0b4a0b5..f620d47 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -721,7 +721,7 @@ libxlDiskSetDiscard(libxl_device_disk *x_disk, int discard)
 if (!x_disk-readwrite)
 return 0;
 #if defined(LIBXL_HAVE_LIBXL_DEVICE_DISK_DISCARD_ENABLE)
-switch ((enum virDomainDiskDiscard)discard) {
+switch ((virDomainDiskDiscard)discard) {
 case VIR_DOMAIN_DISK_DISCARD_DEFAULT:
 case VIR_DOMAIN_DISK_DISCARD_LAST:
 break;
-- 
1.8.4.5

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


Re: [libvirt] [PATCH v2 00/16] Support for per-guest-node binding

2014-07-11 Thread Michal Privoznik

On 08.07.2014 13:50, Martin Kletzander wrote:

Currently we are only able to bind the whole domain to some host nodes
using the /domain/numatune/memory element.  Numerous requests were
made to support host-guest numa node bindings, so this series tries
to implement that using /domain/numatune/memnode elements.

That is incompatible with automatic numa placement (numad) since that
makes no sense.

Some of these patches were ACK'd in the previous round, but this
version completely rewrites the parsing and formatting of the numatune
XML element and places it into a separate file.  Hence the repost of
all the patches even with those ACK'd ones.

Patches 1-3 move some code around, patch 4 adds cell id specification
into the XML (which is used later on).  Then patches 5-7 rework the
numatune handling, which clears out some odd things, but mostly cleans
the parsing code.  Patch 8 adds the support for memnode elements in
the XML conf code and together with patch 9 enables the use of it
outside numatune_conf.c.  After that, I needed to probe qemu for 2
capabilities; for one of them patch 10 adds the possibility to probe
for it, then two following patches 11 and 12 add the probing data.
One of the capabilities tells us that we can use disjoint ranges (not
necessarily for the cpus= param) with qemu, so patch 13 makes a use of
it.  Finally patch 14 uses the memnode data to tell qemu which host
nodes should be used for the allocations of memory blocks.  Patch 15
does almost nothing, but the next patch looks better with it.  And the
last patch, number 16, fixes a bug with KVM and cgroups.

One last note, APIs for handling the memnode settings will be added
later.  I'm sending this to prepare the ground for Michal's work with
hugepages.


Martin Kletzander (16):
   qemu: purely a code movement
   qemu: remove useless error check
   conf: purely a code movement
   conf, schema: add 'id' field for cells
   numatune: create new module for numatune
   numatune: unify numatune struct and enum names
   numatune: Encapsulate numatune configuration in order to unify results
   conf, schema: add support for memnode elements
   numatune: add support for per-node memory bindings in private APIs
   qemu: allow qmp probing for cmdline options without params
   qemu: memory-backend-ram capability probing
   qemu: newer -numa parameter capability probing
   qemu: enable disjoint numa cpu ranges
   qemu: pass numa node binding preferences to qemu
   qemu: split out cpuset.mems setting
   qemu: leave restricting cpuset.mems after initialization

  docs/formatdomain.html.in  |  26 +-
  docs/schemas/domaincommon.rng  |  22 +
  po/POTFILES.in |   1 +
  src/Makefile.am|   3 +-
  src/conf/cpu_conf.c|  39 +-
  src/conf/cpu_conf.h|   3 +-
  src/conf/domain_conf.c | 203 ++-
  src/conf/domain_conf.h |  10 +-
  src/conf/numatune_conf.c   | 589 +
  src/conf/numatune_conf.h   | 108 
  src/libvirt_private.syms   |  24 +-
  src/lxc/lxc_cgroup.c   |  20 +-
  src/lxc/lxc_controller.c   |   5 +-
  src/lxc/lxc_native.c   |  15 +-
  src/parallels/parallels_driver.c   |   7 +-
  src/qemu/qemu_capabilities.c   |  16 +-
  src/qemu/qemu_capabilities.h   |   2 +
  src/qemu/qemu_cgroup.c |  52 +-
  src/qemu/qemu_cgroup.h |   4 +-
  src/qemu/qemu_command.c|  98 +++-
  src/qemu/qemu_driver.c |  84 ++-
  src/qemu/qemu_monitor.c|   6 +-
  src/qemu/qemu_monitor.h|   3 +-
  src/qemu/qemu_monitor_json.c   |   8 +-
  src/qemu/qemu_monitor_json.h   |   3 +-
  src/qemu/qemu_process.c|  12 +-
  src/util/virnuma.c |  61 +--
  src/util/virnuma.h |  28 +-
  tests/qemucapabilitiesdata/caps_1.6.50-1.caps  |   1 +
  tests/qemucapabilitiesdata/caps_1.6.50-1.replies   |   5 +
  tests/qemumonitorjsontest.c|  20 +-
  .../qemuxml2argv-cpu-numa-disjoint.args|   6 +
  .../qemuxml2argv-cpu-numa-disjoint.xml |  28 +
  tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml  |   6 +-
  tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml  |   6 +-
  tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml  |  25 +
  .../qemuxml2argv-numatune-auto-prefer.xml  |  29 +
  .../qemuxml2argv-numatune-memnode-no-memory.args   |   8 +
  .../qemuxml2argv-numatune-memnode-no-memory.xml|  30 ++

Re: [libvirt] [PATCH v2 07/16] numatune: Encapsulate numatune configuration in order to unify results

2014-07-11 Thread Michal Privoznik

On 08.07.2014 13:50, Martin Kletzander wrote:

There were numerous places where numatune configuration (and thus
domain config as well) was changed in different ways.  On some
places this even resulted in persistent domain definition not to be
stable (it would change with daemon's restart).

In order to uniformly change how numatune config is dealt with, all
the internals are now accessible directly only in numatune_conf.c and
outside this file accessors must be used.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
  po/POTFILES.in |   1 +
  src/conf/domain_conf.c | 159 ++-
  src/conf/domain_conf.h |   8 +-
  src/conf/numatune_conf.c   | 316 +
  src/conf/numatune_conf.h   |  72 -
  src/libvirt_private.syms   |  11 +
  src/lxc/lxc_cgroup.c   |  19 +-
  src/lxc/lxc_controller.c   |   5 +-
  src/lxc/lxc_native.c   |  15 +-
  src/parallels/parallels_driver.c   |   7 +-
  src/qemu/qemu_cgroup.c |  23 +-
  src/qemu/qemu_driver.c |  84 +++---
  src/qemu/qemu_process.c|   8 +-
  src/util/virnuma.c |  48 ++--
  src/util/virnuma.h |   2 +-
  .../qemuxml2argv-numatune-auto-prefer.xml  |  29 ++
  .../qemuxml2xmlout-numatune-auto-prefer.xml|  29 ++
  tests/qemuxml2xmltest.c|   2 +
  18 files changed, 553 insertions(+), 285 deletions(-)
  create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.xml
  create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-auto-prefer.xml


Nice.


diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index 14300f0..8b66558 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c



+int
+virDomainNumatuneParseXML(virDomainDefPtr def,
+  xmlXPathContextPtr ctxt)
+{
+char *tmp = NULL;
+int mode = -1;
+int n = 0;
+int placement = -1;
+int ret = -1;
+virBitmapPtr nodeset = NULL;
+xmlNodePtr node = NULL;
+
+if (virXPathInt(count(./numatune), ctxt, n)  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(cannot extract numatune nodes));
+goto cleanup;
+} else if (n  1) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(only one numatune is supported));
+goto cleanup;
+}
+
+node = virXPathNode(./numatune/memory[1], ctxt);
+
+if (def-numatune) {
+virDomainNumatuneFree(def-numatune);
+def-numatune = NULL;
+}
+
+if (!node  def-placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO)
+return 0;
+
+if (!node) {
+/* We know that def-placement_mode is auto if we're here */
+if (virDomainNumatuneSet(def, VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO,
+ -1, NULL)  0)
+goto cleanup;
+return 0;
+}
+
+tmp = virXMLPropString(node, mode);
+if (tmp) {
+mode = virDomainNumatuneMemModeTypeFromString(tmp);
+if (mode  0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(Unsupported NUMA memory tuning mode '%s'),
+   tmp);
+goto cleanup;
+}
+}
+VIR_FREE(tmp);
+
+tmp = virXMLPropString(node, placement);
+if (tmp) {
+placement = virDomainNumatunePlacementTypeFromString(tmp);
+if (placement  0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(Unsupported NUMA memory placement mode '%s'),
+   tmp);
+goto cleanup;
+}
+}
+VIR_FREE(tmp);
+
+tmp = virXMLPropString(node, nodeset);
+if (tmp  virBitmapParse(tmp, 0, nodeset, VIR_DOMAIN_CPUMASK_LEN)  0)
+goto cleanup;
+VIR_FREE(tmp);
+
+if (virDomainNumatuneSet(def, placement, mode, nodeset)  0)


The virDomainNumatuneSet() takes a copy of @nodeset, so you need to call 
virBitmaskFree(nodeset); at the cleanup label.



+goto cleanup;
+
+if (!n) {
+ret = 0;
+goto cleanup;
+}
+
+ret = 0;
+ cleanup:
+VIR_FREE(tmp);
+return ret;
+}
+
+int
+virDomainNumatuneFormatXML(virBufferPtr buf,
+   virDomainNumatunePtr numatune)
+{
+const char *tmp = NULL;


s /const// ..


+
+if (!numatune)
+return 0;
+
+virBufferAddLit(buf, numatune\n);
+virBufferAdjustIndent(buf, 2);
+
+tmp = virDomainNumatuneMemModeTypeToString(numatune-memory.mode);
+virBufferAsprintf(buf, memory mode='%s' , tmp);
+
+if (numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC) {
+   

Re: [libvirt] [PATCH v2 08/16] conf, schema: add support for memnode elements

2014-07-11 Thread Michal Privoznik
On 08.07.2014 13:50, Martin Kletzander wrote:
 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
   docs/formatdomain.html.in  |  15 ++
   docs/schemas/domaincommon.rng  |  17 ++
   src/conf/numatune_conf.c   | 183 
 +++--
   .../qemuxml2argv-numatune-memnode-no-memory.xml|  30 
   .../qemuxml2argv-numatune-memnode-nocpu.xml|  25 +++
   .../qemuxml2argv-numatune-memnode.xml  |  31 
   .../qemuxml2argv-numatune-memnodes-problematic.xml |  31 
   tests/qemuxml2argvtest.c   |   2 +
   .../qemuxml2xmlout-numatune-memnode.xml|  33 
   tests/qemuxml2xmltest.c|   2 +
   10 files changed, 356 insertions(+), 13 deletions(-)
   create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.xml
   create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-nocpu.xml
   create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml
   create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnodes-problematic.xml
   create mode 100644 
 tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-memnode.xml
 
 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
 index ad87b7c..d845c1b 100644
 --- a/docs/formatdomain.html.in
 +++ b/docs/formatdomain.html.in
 @@ -709,6 +709,8 @@
 ...
 lt;numatunegt;
   lt;memory mode=strict nodeset=1-4,^3/gt;
 +lt;memnode cellid=0 mode=strict nodeset=1/gt;
 +lt;memnode cellid=2 mode=preferred nodeset=2/gt;
 lt;/numatunegt;
 ...
   lt;/domaingt;
 @@ -745,6 +747,19 @@
 
   span class='since'Since 0.9.3/span
 /dd
 +  dtcodememnode/code/dt
 +  dd
 +Optional codememnode/code elements can specify memory allocation
 +policies per each guest NUMA node.  For those nodes having no
 +corresponding codememnode/code element, the default from
 +element codememory/code will be used.  Attribute 
 codecellid/code
 +addresses guest NUMA node for which the settings are applied.
 +Attributes codemode/code and codenodeset/code have the same
 +meaning and syntax as in codememory/code element.
 +
 +This setting is not compatible with automatic placement.
 +span class='since'QEMU Since 1.2.6/span

1.2.8 actually

 +  /dd
   /dl
 
 

 
 +static int
 +virDomainNumatuneNodeParseXML(virDomainDefPtr def,
 +  xmlXPathContextPtr ctxt)
 +{
 +int n = 0;;
 +int ret = -1;
 +size_t i = 0;
 +xmlNodePtr *nodes = NULL;
 +
 +if ((n = virXPathNodeSet(./numatune/memnode, ctxt, nodes))  0) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(Cannot extract memnode nodes));
 +goto cleanup;
 +}
 +
 +if (!n)
 +return 0;
 +
 +if (def-numatune  def-numatune-memory.specified 
 +def-numatune-memory.placement == 
 VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO) {
 +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
 +   _(Per-node binding is not compatible with 
 + automatic NUMA placement.));
 +goto cleanup;
 +}
 +
 +if (!def-cpu || !def-cpu-ncells) {
 +virReportError(VIR_ERR_XML_ERROR, %s,
 +   _(Element 'memnode' is invalid without 
 + any guest NUMA cells));
 +goto cleanup;
 +}
 +
 +if (!def-numatune  VIR_ALLOC(def-numatune)  0)
 +goto cleanup;

Here you allow def-numatune to be allocated already.

 +
 +if (VIR_ALLOC_N(def-numatune-mem_nodes, def-cpu-ncells)  0)
 +goto cleanup;

Which means, this can exists too. VIR_REALLOC_N() is safer IMO.

 +
 +def-numatune-nmem_nodes = def-cpu-ncells;
 +
 +for (i = 0; i  n; i++) {
 +const char *tmp = NULL;

No. s/const//.

 +int mode = 0;
 +unsigned int cellid = 0;
 +virDomainNumatuneNodePtr mem_node = NULL;
 +xmlNodePtr cur_node = nodes[i];
 +
 +tmp = virXMLPropString(cur_node, cellid);
 +if (!tmp) {
 +virReportError(VIR_ERR_XML_ERROR, %s,
 +   _(Missing required cellid attribute 
 + in memnode element));
 +goto cleanup;
 +}
 +if (virStrToLong_uip(tmp, NULL, 10, cellid)  0) {
 +virReportError(VIR_ERR_XML_ERROR, %s,
 +   _(Invalid cellid attribute in memnode element));

Moreover, @tmp is leaked here. And it would be nice to tell users in the error 
message @tmp somehow.

 +goto cleanup;
 +}
 +VIR_FREE(tmp);
 +
 +if (cellid = def-numatune-nmem_nodes) {
 +virReportError(VIR_ERR_XML_ERROR, %s,
 +   _(Argument 'cellid' in memnode element must 
 + correspond to existing guest's 

Re: [libvirt] [PATCH v2 04/16] conf, schema: add 'id' field for cells

2014-07-11 Thread Michal Privoznik
On 08.07.2014 13:50, Martin Kletzander wrote:
 In XML format, by definition, order of fields should not matter, so
 order of parsing the elements doesn't affect the end result.  When
 specifying guest NUMA cells, we depend only on the order of the 'cell'
 elements.  With this patch all older domain XMLs are parsed as before,
 but with the 'id' attribute they are parsed and formatted according to
 that field.  This will be useful when we have tuning settings for
 particular guest NUMA node.
 
 Signed-off-by: Martin Kletzander mklet...@redhat.com
 ---
   docs/formatdomain.html.in  | 11 +++---
   docs/schemas/domaincommon.rng  |  5 +++
   src/conf/cpu_conf.c| 39 
 +++---
   src/conf/cpu_conf.h|  3 +-
   src/qemu/qemu_command.c|  2 +-
   tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml  |  6 ++--
   tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml  |  6 ++--
   tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml  | 25 ++
   tests/qemuxml2argvtest.c   |  1 +
   .../qemuxml2xmlout-cpu-numa1.xml   | 28 
   .../qemuxml2xmlout-cpu-numa2.xml   | 28 
   tests/qemuxml2xmltest.c|  3 ++
   12 files changed, 139 insertions(+), 18 deletions(-)
   create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml
   create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa1.xml
   create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa2.xml
 
 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
 index b69da4c..ad87b7c 100644
 --- a/docs/formatdomain.html.in
 +++ b/docs/formatdomain.html.in
 @@ -1030,8 +1030,8 @@
 lt;cpugt;
   ...
   lt;numagt;
 -  lt;cell cpus='0-3' memory='512000'/gt;
 -  lt;cell cpus='4-7' memory='512000'/gt;
 +  lt;cell id='0' cpus='0-3' memory='512000'/gt;
 +  lt;cell id='1' cpus='4-7' memory='512000'/gt;
   lt;/numagt;
   ...
 lt;/cpugt;
 @@ -1041,8 +1041,11 @@
 Each codecell/code element specifies a NUMA cell or a NUMA node.
 codecpus/code specifies the CPU or range of CPUs that are part of
 the node. codememory/code specifies the node memory in kibibytes
 -  (i.e. blocks of 1024 bytes). Each cell or node is assigned cellid
 -  or nodeid in the increasing order starting from 0.
 +  (i.e. blocks of 1024 bytes). All cells should have codeid/code
 +  attribute in case referring to some cell is necessary in the code,
 +  otherwise the cells are assigned ids in the increasing order starting
 +  from 0.  Mixing cells with and without the codeid/code attribute
 +  is not recommended as it may result in unwanted behaviour.

I'd note here, that the @id attribute is since 1.2.7

   /p
 
   p
 diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
 index 7be028d..155a33e 100644
 --- a/docs/schemas/domaincommon.rng
 +++ b/docs/schemas/domaincommon.rng
 @@ -3902,6 +3902,11 @@
 
 define name=numaCell
   element name=cell
 +  optional
 +attribute name=id
 +  ref name=unsignedInt/
 +/attribute
 +  /optional
 attribute name=cpus
   ref name=cpuset/
 /attribute
 diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
 index 811893d..9e0af08 100644
 --- a/src/conf/cpu_conf.c
 +++ b/src/conf/cpu_conf.c
 @@ -152,7 +152,6 @@ virCPUDefCopy(const virCPUDef *cpu)
   copy-ncells_max = copy-ncells = cpu-ncells;
 
   for (i = 0; i  cpu-ncells; i++) {
 -copy-cells[i].cellid = cpu-cells[i].cellid;
   copy-cells[i].mem = cpu-cells[i].mem;
 
   copy-cells[i].cpumask = 
 virBitmapNewCopy(cpu-cells[i].cpumask);
 @@ -438,17 +437,46 @@ virCPUDefParseXML(xmlNodePtr node,
   for (i = 0; i  n; i++) {
   char *cpus, *memory;
   int ret, ncpus = 0;
 +unsigned int cur_cell;
 +char *tmp = NULL;
 +
 +tmp = virXMLPropString(nodes[i], id);
 +if (!tmp) {
 +cur_cell = i;
 +} else {
 +ret  = virStrToLong_ui(tmp, NULL, 10, cur_cell);
 +VIR_FREE(tmp);
 +if (ret == -1) {
 +virReportError(VIR_ERR_XML_ERROR, %s,
 +   _(Invalid 'id' attribute in NUMA cell));
 +goto error;
 +}
 +}

If there's a typo in the @id, I think this can make users lives easier:

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index 9e0af08..5003cf1 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -445,12 +445,14 @@ virCPUDefParseXML(xmlNodePtr node,
 cur_cell = i;
 } else {
 ret  = virStrToLong_ui(tmp, NULL, 10, cur_cell);

Re: [libvirt] [PATCH v2 09/16] numatune: add support for per-node memory bindings in private APIs

2014-07-11 Thread Michal Privoznik

On 08.07.2014 13:50, Martin Kletzander wrote:

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
  src/conf/numatune_conf.c | 111 ---
  src/conf/numatune_conf.h |  14 --
  src/libvirt_private.syms |   1 +
  src/lxc/lxc_cgroup.c |   3 +-
  src/qemu/qemu_cgroup.c   |   2 +-
  src/qemu/qemu_driver.c   |  10 ++---
  src/util/virnuma.c   |   6 +--
  7 files changed, 117 insertions(+), 30 deletions(-)

diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index 67fc799..60b6867 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c
@@ -63,6 +63,18 @@ struct _virDomainNumatune {
  };


+static inline bool
+numa_cell_specified(virDomainNumatunePtr numatune,


Whoa, when I met this function call I thought to myself that this is 
some libnuma function. Please name it differently to match our 
virSomethingShiny pattern.



+int cellid)
+{
+if (numatune 
+cellid = 0 
+cellid  numatune-nmem_nodes)
+return numatune-mem_nodes[cellid].nodeset;
+
+return false;
+}
+
  static int
  virDomainNumatuneNodeParseXML(virDomainDefPtr def,
xmlXPathContextPtr ctxt)
@@ -312,6 +324,7 @@ void
  virDomainNumatuneFree(virDomainNumatunePtr numatune)
  {
  size_t i = 0;
+


This change is spurious. Either move it to 8/16 or drop this one.


  if (!numatune)
  return;

@@ -324,26 +337,37 @@ virDomainNumatuneFree(virDomainNumatunePtr numatune)
  }

  virDomainNumatuneMemMode
-virDomainNumatuneGetMode(virDomainNumatunePtr numatune)
+virDomainNumatuneGetMode(virDomainNumatunePtr numatune,
+ int cellid)
  {
-return (numatune  numatune-memory.specified) ? numatune-memory.mode : 
0;
+if (!numatune)
+return 0;
+
+if (numa_cell_specified(numatune, cellid))
+return numatune-mem_nodes[cellid].mode;
+
+if (numatune-memory.specified)
+return numatune-memory.mode;
+
+return 0;
  }

  virBitmapPtr
  virDomainNumatuneGetNodeset(virDomainNumatunePtr numatune,
-virBitmapPtr auto_nodeset)
+virBitmapPtr auto_nodeset,
+int cellid)
  {
  if (!numatune)
  return NULL;

-if (numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO)
+if (numatune-memory.specified 
+numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO)
  return auto_nodeset;

-/*
- * This weird logic has the same meaning as switch with
- * auto/static/default, but can be more readably changed later.
- */
-if (numatune-memory.placement != VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC)
+if (numa_cell_specified(numatune, cellid))
+return numatune-mem_nodes[cellid].nodeset;
+
+if (!numatune-memory.specified)
  return NULL;

  return numatune-memory.nodeset;
@@ -351,23 +375,30 @@ virDomainNumatuneGetNodeset(virDomainNumatunePtr numatune,

  char *
  virDomainNumatuneFormatNodeset(virDomainNumatunePtr numatune,
-   virBitmapPtr auto_nodeset)
+   virBitmapPtr auto_nodeset,
+   int cellid)
  {
  return virBitmapFormat(virDomainNumatuneGetNodeset(numatune,
-   auto_nodeset));
+   auto_nodeset,
+   cellid));
  }

  int
  virDomainNumatuneMaybeFormatNodeset(virDomainNumatunePtr numatune,
  virBitmapPtr auto_nodeset,
-char **mask)
+char **mask,
+int cellid)
  {
  *mask = NULL;

  if (!numatune)
  return 0;

-if (numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO 
+if (!numa_cell_specified(numatune, cellid)  !numatune-memory.specified)
+return 0;
+
+if (numatune-memory.specified 
+numatune-memory.placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO 
  !auto_nodeset) {
  virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 _(Advice from numad is needed in case of 
@@ -375,7 +406,7 @@ virDomainNumatuneMaybeFormatNodeset(virDomainNumatunePtr 
numatune,
  return -1;
  }

-*mask = virDomainNumatuneFormatNodeset(numatune, auto_nodeset);
+*mask = virDomainNumatuneFormatNodeset(numatune, auto_nodeset, cellid);
  if (!*mask)
  return -1;

@@ -469,6 +500,35 @@ virDomainNumatuneSet(virDomainDefPtr def,
  return ret;
  }

+static bool
+virDomainNumatuneNodesEqual(virDomainNumatunePtr n1,
+virDomainNumatunePtr n2)
+{
+size_t i = 0;
+
+if (n1-nmem_nodes != n2-nmem_nodes)
+return false;
+
+for (i = 0; i  n1-nmem_nodes; i++) {
+

Re: [libvirt] [PATCH v2 14/16] qemu: pass numa node binding preferences to qemu

2014-07-11 Thread Michal Privoznik

On 08.07.2014 13:50, Martin Kletzander wrote:

Currently, we only bind the whole QEMU domain to memory nodes
specified in nodemask altogether.  That, however, doesn't make much
sense when one wants to control from where the memory for particular
guest nodes should be allocated.  QEMU allows us to do that by
specifying 'host-nodes' parameter for the 'memory-backend-ram' object,
so let's use that.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
  src/qemu/qemu_command.c| 59 +-
  .../qemuxml2argv-numatune-memnode-no-memory.args   |  8 +++
  .../qemuxml2argv-numatune-memnode.args | 11 
  .../qemuxml2argv-numatune-memnode.xml  | 14 ++---
  tests/qemuxml2argvtest.c   |  7 +++
  5 files changed, 92 insertions(+), 7 deletions(-)
  create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode-no-memory.args
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.args




diff --git a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml
index 18b00d8..49b328c 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-numatune-memnode.xml
@@ -1,12 +1,13 @@
  domain type='qemu'
nameQEMUGuest/name
uuid9f4b6512-e73a-4a25-93e8-5307802821ce/uuid
-  memory unit='KiB'65536/memory
-  currentMemory unit='KiB'65536/currentMemory
-  vcpu placement='static'2/vcpu
+  memory unit='KiB'24682468/memory
+  currentMemory unit='KiB'24682468/currentMemory
+  vcpu placement='static'32/vcpu
numatune
-memory mode='strict' nodeset='0-3'/
+memory mode='strict' nodeset='0-7'/
  memnode cellid='0' mode='preferred' nodeset='3'/
+memnode cellid='2' mode='strict' nodeset='1-2,5-7,^6'/
/numatune
os
  type arch='x86_64' machine='pc'hvm/type
@@ -14,8 +15,9 @@
/os
cpu
  numa
-  cell id='0' cpus='0' memory='32768'/
-  cell id='1' cpus='1' memory='32768'/
+  cell id='0' cpus='0' memory='20002'/
+  cell id='1' cpus='1-27,29' memory='660066'/
+  cell id='2' cpus='28-31,^29' memory='24002400'/


AHA! This explain why I'm seeing the test error in 8/16. Something went 
wrong during the rebase I think. Because now I have to revert the squash 
in from 8/16 to make the test work again. Yes, it's failing now.



  /numa
/cpu
clock offset='utc'/


Unfortunately, I can't ACK neither this one (same as I couldn't 8/16).

Michal

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


Re: [libvirt] [PATCH 1/2] Don't output libvirt-UUID.files for LXC apparmor profiles

2014-07-11 Thread Serge Hallyn
Quoting Cédric Bosdonnat (cbosdon...@suse.com):
 ---
  src/security/virt-aa-helper.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

Hi,

I'm acking this anyway bc I think you're right, but I'm trying to
think of a case where this would still be useful.  What if we want
to allow only a certain container to have access to its cgroups,
for instance, for nesting containers.  Would virt-aa-helper and the
.files be a way this would be done?  I suppose we coudl always re-introduce
this in that case...  

Acked-by: Serge E. Hallyn serge.hal...@ubuntu.com

 
 diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
 index b5f66f3..d563b98 100644
 --- a/src/security/virt-aa-helper.c
 +++ b/src/security/virt-aa-helper.c
 @@ -1342,7 +1342,8 @@ main(int argc, char **argv)
  vah_info(include_file);
  vah_info(included_files);
  rc = 0;
 -} else if ((rc = update_include_file(include_file,
 +} else if (ctl-def-virtType != VIR_DOMAIN_VIRT_LXC 
 +   (rc = update_include_file(include_file,
   included_files,
   ctl-append)) != 0)
  goto cleanup;
 -- 
 1.8.4.5
 
 --
 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 2/2] Rework lxc apparmor profile

2014-07-11 Thread Serge Hallyn
Quoting Cédric Bosdonnat (cbosdon...@suse.com):
 Rework the apparmor lxc profile abstraction to mimic ubuntu's 
 container-default.
 This profile allows quite a lot, but strives to restrict access to
 dangerous resources.
 
 Removing the explicit authorizations to bash, systemd and cron files,
 forces them to keep the lxc profile for all applications inside the
 container. PUx permissions where leading to running systemd (and others
 tasks) unconfined.
 
 Put the generic files, network and capabilities restrictions directly
 in the TEMPLATE.lxc: this way, users can restrict them on a per
 container basis.
 ---
  examples/apparmor/Makefile.am |   6 +-
  examples/apparmor/TEMPLATE.lxc|  15 
  examples/apparmor/{TEMPLATE = TEMPLATE.qemu} |   2 +-
  examples/apparmor/libvirt-lxc | 119 
 +++---
  src/security/security_apparmor.c  |  20 +++--
  src/security/virt-aa-helper.c |  29 +--
  6 files changed, 148 insertions(+), 43 deletions(-)
  create mode 100644 examples/apparmor/TEMPLATE.lxc
  rename examples/apparmor/{TEMPLATE = TEMPLATE.qemu} (75%)
 
 diff --git a/examples/apparmor/Makefile.am b/examples/apparmor/Makefile.am
 index a741e94..7a20e16 100644
 --- a/examples/apparmor/Makefile.am
 +++ b/examples/apparmor/Makefile.am
 @@ -15,7 +15,8 @@
  ## http://www.gnu.org/licenses/.
  
  EXTRA_DIST=  \
 - TEMPLATE\
 + TEMPLATE.qemu   \
 + TEMPLATE.lxc\
   libvirt-qemu\
   libvirt-lxc \
   usr.lib.libvirt.virt-aa-helper  \
 @@ -36,6 +37,7 @@ abstractions_DATA = \
  
  templatesdir = $(apparmordir)/libvirt
  templates_DATA = \
 - TEMPLATE \
 + TEMPLATE.qemu \
 + TEMPLATE.lxc \
   $(NULL)
  endif WITH_APPARMOR_PROFILES
 diff --git a/examples/apparmor/TEMPLATE.lxc b/examples/apparmor/TEMPLATE.lxc
 new file mode 100644
 index 000..7b64885
 --- /dev/null
 +++ b/examples/apparmor/TEMPLATE.lxc
 @@ -0,0 +1,15 @@
 +#
 +# This profile is for the domain whose UUID matches this file.
 +#
 +
 +#include tunables/global
 +
 +profile LIBVIRT_TEMPLATE {
 +  #include abstractions/libvirt-lxc
 +
 +  # Globally allows everything to run under this profile
 +  # These can be narrowed depending on the container's use.
 +  file,
 +  capability,
 +  network,
 +}
 diff --git a/examples/apparmor/TEMPLATE b/examples/apparmor/TEMPLATE.qemu
 similarity index 75%
 rename from examples/apparmor/TEMPLATE
 rename to examples/apparmor/TEMPLATE.qemu
 index 187dec5..008a221 100644
 --- a/examples/apparmor/TEMPLATE
 +++ b/examples/apparmor/TEMPLATE.qemu
 @@ -5,5 +5,5 @@
  #include tunables/global
  
  profile LIBVIRT_TEMPLATE {
 -  #include abstractions/libvirt-driver
 +  #include abstractions/libvirt-qemu
  }
 diff --git a/examples/apparmor/libvirt-lxc b/examples/apparmor/libvirt-lxc
 index d404328..4bfb503 100644
 --- a/examples/apparmor/libvirt-lxc
 +++ b/examples/apparmor/libvirt-lxc
 @@ -2,16 +2,115 @@
  
#include abstractions/base
  
 -  # Needed for lxc-enter-namespace
 -  capability sys_admin,
 -  capability sys_chroot,
 +  umount,
  
 -  # Added for lxc-enter-namespace --cmd /bin/bash
 -  /bin/bash PUx,
 +  # ignore DENIED message on / remount
 +  deny mount options=(ro, remount) - /,
  
 -  /usr/sbin/cron PUx,
 -  /usr/lib/systemd/systemd PUx,
 +  # allow tmpfs mounts everywhere
 +  mount fstype=tmpfs,
  
 -  /usr/lib/libsystemd-*.so.* mr,
 -  /usr/lib/libudev-*.so.* mr,
 -  /etc/ld.so.cache mr,
 +  # allow mqueue mounts everywhere
 +  mount fstype=mqueue,
 +
 +  # allow fuse mounts everywhere
 +  mount fstype=fuse.*,
 +
 +  # deny writes in /proc/sys/fs but allow binfmt_misc to be mounted
 +  mount fstype=binfmt_misc - /proc/sys/fs/binfmt_misc/,
 +  deny @{PROC}/sys/fs/** wklx,
 +
 +  # allow efivars to be mounted, writing to it will be blocked though
 +  mount fstype=efivarfs - /sys/firmware/efi/efivars/,
 +
 +  # block some other dangerous paths
 +  deny @{PROC}/sysrq-trigger rwklx,
 +  deny @{PROC}/mem rwklx,
 +  deny @{PROC}/kmem rwklx,
 +
 +  # deny writes in /sys except for /sys/fs/cgroup, also allow
 +  # fusectl, securityfs and debugfs to be mounted there (read-only)
 +  mount fstype=fusectl - /sys/fs/fuse/connections/,
 +  mount fstype=securityfs - /sys/kernel/security/,
 +  mount fstype=debugfs - /sys/kernel/debug/,
 +  mount fstype=proc - /proc/,
 +  mount fstype=sysfs - /sys/,
 +  deny /sys/firmware/efi/efivars/** rwklx,
 +  deny /sys/kernel/security/** rwklx,
 +
 +  # generated by: lxc-generate-aa-rules.py container-rules.base
 +  deny /proc/sys/[^kn]*{,/**} wklx,
 +  deny /proc/sys/k[^e]*{,/**} wklx,
 +  deny /proc/sys/ke[^r]*{,/**} wklx,
 +  deny /proc/sys/ker[^n]*{,/**} wklx,
 +  deny /proc/sys/kern[^e]*{,/**} wklx,
 +  deny /proc/sys/kerne[^l]*{,/**} wklx,
 +  deny /proc/sys/kernel/[^smhd]*{,/**} wklx,
 +  deny /proc/sys/kernel/d[^o]*{,/**} wklx,
 +  deny 

[libvirt] [PATCH] conf: Fix possible NULL dereference in virStorageVolTargetDefFormat

2014-07-11 Thread Matthias Bolte
Commit dae1568c6c6455091e8cd9bc2e90a22af3d3880c converted the perms
member of the virStorageVolTarget struct into a pointer to make it
optional. But virStorageVolTargetDefFormat did not check perms for
NULL before dereferencing it.
---
 src/conf/storage_conf.c | 26 ++
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 9ac5975..aa29658 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1423,22 +1423,24 @@ virStorageVolTargetDefFormat(virStorageVolOptionsPtr 
options,
 virBufferAsprintf(buf, format type='%s'/\n, format);
 }
 
-virBufferAddLit(buf, permissions\n);
-virBufferAdjustIndent(buf, 2);
+if (def-perms) {
+virBufferAddLit(buf, permissions\n);
+virBufferAdjustIndent(buf, 2);
 
-virBufferAsprintf(buf, mode0%o/mode\n,
-  def-perms-mode);
-virBufferAsprintf(buf, owner%u/owner\n,
-  (unsigned int) def-perms-uid);
-virBufferAsprintf(buf, group%u/group\n,
-  (unsigned int) def-perms-gid);
+virBufferAsprintf(buf, mode0%o/mode\n,
+  def-perms-mode);
+virBufferAsprintf(buf, owner%u/owner\n,
+  (unsigned int) def-perms-uid);
+virBufferAsprintf(buf, group%u/group\n,
+  (unsigned int) def-perms-gid);
 
 
-virBufferEscapeString(buf, label%s/label\n,
-  def-perms-label);
+virBufferEscapeString(buf, label%s/label\n,
+  def-perms-label);
 
-virBufferAdjustIndent(buf, -2);
-virBufferAddLit(buf, /permissions\n);
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, /permissions\n);
+}
 
 if (def-timestamps) {
 virBufferAddLit(buf, timestamps\n);
-- 
1.9.1

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


Re: [libvirt] [PATCHv2 0/2] doc: Improve snapshot/blockjob docs

2014-07-11 Thread Eric Blake
On 07/11/2014 03:01 AM, Peter Krempa wrote:
 Peter Krempa (2):
   doc: Document that snapshot name of block-backed disk isnt
 autogenerated
   doc: Be more specific about semantics of _REUSE_EXT flag

ACK series; you picked up on all my suggestions on v1.

 
  docs/formatsnapshot.html.in |  8 +---
  src/libvirt.c   | 23 ++-
  tools/virsh.pod | 21 +
  3 files changed, 32 insertions(+), 20 deletions(-)
 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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 2/3] virSecurityLabelDefParseXML: Rework

2014-07-11 Thread Eric Blake
On 07/11/2014 03:32 AM, Michal Privoznik wrote:
 Instead of allocating the virSecurityLabelDef structure ourselves, we
 can utilize virSecurityLabelDefNew which even sets the default values
 for us.
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  src/conf/domain_conf.c | 103 
 -
  1 file changed, 50 insertions(+), 53 deletions(-)
 
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 7b90903..de60cd2 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -4553,91 +4553,87 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt,
  unsigned int flags)
  {
  char *p;
 -virSecurityLabelDefPtr def = NULL;
 +virSecurityLabelDefPtr seclabel = NULL;

Looks like you did a search-and-replace s/def/seclabel/...

  
 -if (VIR_ALLOC(def)  0)
 +p = virXPathStringLimit(string(./@model),
 +VIR_SECURITY_MODEL_BUFLEN - 1, ctxt);
 +
 +if (!(seclabel = virSecurityLabelDefNew(p)))
  goto error;
  
 +/* set seclabelault value */

...but didn't pay attention to what got replaced :)


 -if (p != NULL) {
 +if (p) {
  if (STREQ(p, yes)) {
 -def-relabel = true;
 +seclabel-relabel = true;

I'm guessing that this assignment is now redundant with the fact that it
already defaults to this value; but I'm okay leaving it to make the code
obvious.

ACK with the typo fix.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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 v3 1/3] conf: Always format seclabel's model

2014-07-11 Thread Eric Blake
On 07/11/2014 03:32 AM, Michal Privoznik wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1113860
 
 We've always done that. Well, until 990e46c45. Point is, if we don't
 format model, we may lose a domain on libvirtd restart. If the
 seclabel is implicit however, we should skip it's formatting.
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  src/conf/domain_conf.c | 34 
 +++---
  .../qemuxml2argv-seclabel-dynamic-none.xml | 28 ++
  tests/qemuxml2xmltest.c|  1 +
  3 files changed, 52 insertions(+), 11 deletions(-)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-none.xml
 

ACK

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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 3/3] virSecurityLabelDef: use enum type for @type

2014-07-11 Thread Eric Blake
On 07/11/2014 03:32 AM, Michal Privoznik wrote:
 There's this trend in libvirt of using enum types wherever possible.
 Now that I'm at virSecurityLabelDef let's rework @type item of the
 structure so we don't have to typecast it elsewhere.
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  src/conf/domain_conf.c  | 6 --
  src/security/security_dac.c | 2 +-
  src/util/virseclabel.h  | 2 +-
  3 files changed, 6 insertions(+), 4 deletions(-)

I'm not quite as sure about this one.  This solves the issue of how to
detect errors when parsing strings to enum, but required the use of an
intermediate variable which in turn made the patch a net gain in lines
of code.  If someone forgets to use the intermediate variable for
parsing, this backfires.  On the other hand, parsing string to enum
should be done in just one location, and that's the location touched by
this patch.  I'm 50-50 on whether to take this, so I'd like someone else
to chime in with an opinion.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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] conf: Fix possible NULL dereference in virStorageVolTargetDefFormat

2014-07-11 Thread Eric Blake
On 07/11/2014 10:13 AM, Matthias Bolte wrote:
 Commit dae1568c6c6455091e8cd9bc2e90a22af3d3880c converted the perms
 member of the virStorageVolTarget struct into a pointer to make it
 optional. But virStorageVolTargetDefFormat did not check perms for
 NULL before dereferencing it.
 ---
  src/conf/storage_conf.c | 26 ++
  1 file changed, 14 insertions(+), 12 deletions(-)

ACK.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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 1/2] Don't output libvirt-UUID.files for LXC apparmor profiles

2014-07-11 Thread Eric Blake
On 07/11/2014 09:22 AM, Serge Hallyn wrote:
 Quoting Cédric Bosdonnat (cbosdon...@suse.com):
 ---
  src/security/virt-aa-helper.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
 
 Hi,
 
 I'm acking this anyway bc I think you're right, but I'm trying to
 think of a case where this would still be useful.  What if we want
 to allow only a certain container to have access to its cgroups,
 for instance, for nesting containers.  Would virt-aa-helper and the
 .files be a way this would be done?  I suppose we coudl always re-introduce
 this in that case...  
 
 Acked-by: Serge E. Hallyn serge.hal...@ubuntu.com

I've pushed this one.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PREPOST 05/17] src/xenxs:Refactor PCI parsing code

2014-07-11 Thread Jim Fehlig
David Kiarie wrote:
 From: Kiarie Kahurani davidkiar...@gmail.com

 Introduce the function
  xenParseXMPCI(.);
 which parses PCI config

 signed-off-by:David Kiarie davidkiar...@gmail.com
 ---
  src/xenxs/xen_xm.c | 192 
 +++--
  1 file changed, 99 insertions(+), 93 deletions(-)

 diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
 index e30ab9c..7a5b47c 100644
 --- a/src/xenxs/xen_xm.c
 +++ b/src/xenxs/xen_xm.c
 @@ -529,6 +529,102 @@ static int xenParseXMEventsActions(virConfPtr conf, 
 virDomainDefPtr def)
  return 0;
  
  }
 +static int
 +xenParseXMPCI(virConfPtr conf, virDomainDefPtr def)
 +{
 +virConfValuePtr list = virConfGetValue(conf, pci);
 +virDomainHostdevDefPtr hostdev = NULL;
   

Blank line after local variables.

 +if (list  list-type == VIR_CONF_LIST) {
 +list = list-list;
 +while (list) {
 +char domain[5];
 +char bus[3];
 +char slot[3];
 +char func[2];
 +char *key, *nextkey;
 +int domainID;
 +int busID;
 +int slotID;
 +int funcID;
 +
 +domain[0] = bus[0] = slot[0] = func[0] = '\0';
 +
 +if ((list-type != VIR_CONF_STRING) || (list-str == NULL))
 +goto skippci;
 +
 +/* pci=[':00:1b.0',':00:13.0'] */
 +if (!(key = list-str))
 +goto skippci;
 +if (!(nextkey = strchr(key, ':')))
 +goto skippci;
 +
 +if (virStrncpy(domain, key, (nextkey - key), sizeof(domain)) == 
 NULL) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(Domain %s too big for destination), key);
   

Pre-existing, but while we are touching the code, I wonder if the use of
virReportError here is correct?  The device is skipped if there is a
problem parsing it.  I think these errors should be logged via VIR_WARN,
but would like confirmation from another libvirt dev before asking you
to change them.  At the very least, the error should be changed to
VIR_ERR_CONF_SYNTAX.

Regards,
Jim

 +goto skippci;
 +}
 +
 +key = nextkey + 1;
 +if (!(nextkey = strchr(key, ':')))
 +goto skippci;
 +
 +if (virStrncpy(bus, key, (nextkey - key), sizeof(bus)) == NULL) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(Bus %s too big for destination), key);
 +goto skippci;
 +}
 +
 +key = nextkey + 1;
 +if (!(nextkey = strchr(key, '.')))
 +goto skippci;
 +
 +if (virStrncpy(slot, key, (nextkey - key), sizeof(slot)) == 
 NULL) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(Slot %s too big for destination), key);
 +goto skippci;
 +}
 +
 +key = nextkey + 1;
 +if (strlen(key) != 1)
 +goto skippci;
 +
 +if (virStrncpy(func, key, 1, sizeof(func)) == NULL) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(Function %s too big for destination), 
 key);
 +goto skippci;
 +}
 +
 +if (virStrToLong_i(domain, NULL, 16, domainID)  0)
 +goto skippci;
 +if (virStrToLong_i(bus, NULL, 16, busID)  0)
 +goto skippci;
 +if (virStrToLong_i(slot, NULL, 16, slotID)  0)
 +goto skippci;
 +if (virStrToLong_i(func, NULL, 16, funcID)  0)
 +goto skippci;
 +
 +if (!(hostdev = virDomainHostdevDefAlloc()))
 +   return -1;
 +
 +hostdev-managed = false;
 +hostdev-source.subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI;
 +hostdev-source.subsys.u.pci.addr.domain = domainID;
 +hostdev-source.subsys.u.pci.addr.bus = busID;
 +hostdev-source.subsys.u.pci.addr.slot = slotID;
 +hostdev-source.subsys.u.pci.addr.function = funcID;
 +
 +if (VIR_APPEND_ELEMENT(def-hostdevs, def-nhostdevs, hostdev)  
 0) {
 +virDomainHostdevDefFree(hostdev);
 +return -1;
 +}
 +
 +skippci:
 +list = list-next;
 +}
 +}
 +
 +return 0;
 +}
  virDomainDefPtr
  xenParseXM(virConfPtr conf, int xendConfigVersion,
 virCapsPtr caps)
 @@ -541,7 +637,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  virDomainDiskDefPtr disk = NULL;
  virDomainNetDefPtr net = NULL;
  virDomainGraphicsDefPtr graphics = NULL;
 -virDomainHostdevDefPtr hostdev = NULL;
  size_t i;
  unsigned long count;
  char *script = NULL;
 @@ -848,98 +943,9 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
 goto cleanup;
 if 

Re: [libvirt] [PREPOST 06/17] src/xenxs:Refactor code parsing CPU config

2014-07-11 Thread Jim Fehlig
David Kiarie wrote:
 From: Kiarie Kahurani davidkiar...@gmail.com

 Introduce xenParseXMCPUFeatures(.);
 This function parses config related to CPU and power
 management

 signed-off-by:David Kiariedavidkiar...@gmail.com
 ---
  src/xenxs/xen_xm.c | 116 
 -
  1 file changed, 62 insertions(+), 54 deletions(-)

 diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
 index 7a5b47c..ebeeeb5 100644
 --- a/src/xenxs/xen_xm.c
 +++ b/src/xenxs/xen_xm.c
 @@ -625,6 +625,66 @@ xenParseXMPCI(virConfPtr conf, virDomainDefPtr def)
  
  return 0;
  }
 +static int xenParseXMCPUFeatures(virConfPtr conf, virDomainDefPtr def)
 +{
 +unsigned long count;
 +const char *str;
 +int val;
   

The usual whitespace comments, but otherwise looks good.

Regards,
Jim

 +if (xenXMConfigGetULong(conf, vcpus, count, 1)  0 ||
 +MAX_VIRT_CPUS  count)
 +return -1;
 +def-maxvcpus = count;
 +if (xenXMConfigGetULong(conf, vcpu_avail, count, -1)  0)
 +return -1;
 +def-vcpus = MIN(count_one_bits_l(count), def-maxvcpus);
 +
 +if (xenXMConfigGetString(conf, cpus, str, NULL)  0)
 +return -1;
 +if (str  (virBitmapParse(str, 0, def-cpumask, 4096)  0))
 +return -1;
 +
 +if (STREQ(def-os.type, hvm)) {
 +if (xenXMConfigGetBool(conf, pae, val, 0)  0)
 +return -1;
 +else if (val)
 +def-features[VIR_DOMAIN_FEATURE_PAE] = 
 VIR_DOMAIN_FEATURE_STATE_ON;
 +if (xenXMConfigGetBool(conf, acpi, val, 0)  0)
 +return -1;
 +else if (val)
 +def-features[VIR_DOMAIN_FEATURE_ACPI] = 
 VIR_DOMAIN_FEATURE_STATE_ON;
 +if (xenXMConfigGetBool(conf, apic, val, 0)  0)
 +return -1;
 +else if (val)
 +def-features[VIR_DOMAIN_FEATURE_APIC] = 
 VIR_DOMAIN_FEATURE_STATE_ON;
 +if (xenXMConfigGetBool(conf, hap, val, 0)  0)
 +return -1;
 +else if (val)
 +def-features[VIR_DOMAIN_FEATURE_HAP] = 
 VIR_DOMAIN_FEATURE_STATE_ON;
 +if (xenXMConfigGetBool(conf, viridian, val, 0)  0)
 +return -1;
 +else if (val)
 +def-features[VIR_DOMAIN_FEATURE_VIRIDIAN] = 
 VIR_DOMAIN_FEATURE_STATE_ON;
 +
 +if (xenXMConfigGetBool(conf, hpet, val, -1)  0)
 +return -1;
 +else if (val != -1) {
 +virDomainTimerDefPtr timer;
 +
 +if (VIR_ALLOC_N(def-clock.timers, 1)  0 ||
 +VIR_ALLOC(timer)  0)
 +return -1;
 +
 +timer-name = VIR_DOMAIN_TIMER_NAME_HPET;
 +timer-present = val;
 +timer-tickpolicy = -1;
 +
 +def-clock.ntimers = 1;
 +def-clock.timers[0] = timer;
 +}
 +}
 +
 +return 0;
 +}
  virDomainDefPtr
  xenParseXM(virConfPtr conf, int xendConfigVersion,
 virCapsPtr caps)
 @@ -638,7 +698,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  virDomainNetDefPtr net = NULL;
  virDomainGraphicsDefPtr graphics = NULL;
  size_t i;
 -unsigned long count;
  char *script = NULL;
  char *listenAddr = NULL;
  
 @@ -703,59 +762,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  }
  if (xenParseXMMem(conf, def)  0)
  goto cleanup;
 -
 -if (xenXMConfigGetULong(conf, vcpus, count, 1)  0 ||
 -MAX_VIRT_CPUS  count)
 -goto cleanup;
 -def-maxvcpus = count;
 -if (xenXMConfigGetULong(conf, vcpu_avail, count, -1)  0)
 -goto cleanup;
 -def-vcpus = MIN(count_one_bits_l(count), def-maxvcpus);
 -
 -if (xenXMConfigGetString(conf, cpus, str, NULL)  0)
 -goto cleanup;
 -if (str  (virBitmapParse(str, 0, def-cpumask, 4096)  0))
 -goto cleanup;
 -
 -if (hvm) {
 -if (xenXMConfigGetBool(conf, pae, val, 0)  0)
 -goto cleanup;
 -else if (val)
 -def-features[VIR_DOMAIN_FEATURE_PAE] = 
 VIR_DOMAIN_FEATURE_STATE_ON;
 -if (xenXMConfigGetBool(conf, acpi, val, 0)  0)
 -goto cleanup;
 -else if (val)
 -def-features[VIR_DOMAIN_FEATURE_ACPI] = 
 VIR_DOMAIN_FEATURE_STATE_ON;
 -if (xenXMConfigGetBool(conf, apic, val, 0)  0)
 -goto cleanup;
 -else if (val)
 -def-features[VIR_DOMAIN_FEATURE_APIC] = 
 VIR_DOMAIN_FEATURE_STATE_ON;
 -if (xenXMConfigGetBool(conf, hap, val, 0)  0)
 -goto cleanup;
 -else if (val)
 -def-features[VIR_DOMAIN_FEATURE_HAP] = 
 VIR_DOMAIN_FEATURE_STATE_ON;
 -if (xenXMConfigGetBool(conf, viridian, val, 0)  0)
 -goto cleanup;
 -else if (val)
 -def-features[VIR_DOMAIN_FEATURE_VIRIDIAN] = 
 VIR_DOMAIN_FEATURE_STATE_ON;
 -
 -if (xenXMConfigGetBool(conf, hpet, val, -1)  0)
 -goto cleanup;
 -else if (val != -1) {
 -virDomainTimerDefPtr timer;
 -
 -if 

[libvirt] [PATCH v2] support for QEMU vhost-user

2014-07-11 Thread Michele Paolino
This patch adds support for the QEMU vhost-user feature to libvirt.
vhost-user enables the communication between a QEMU virtual machine
and other userspace process using the Virtio transport protocol.
It uses a char dev (e.g. Unix socket) for the control plane,
while the data plane based on shared memory.

The XML looks like:

interface type='vhostuser'
source type='unix' path='/tmp/vhost.sock' mode='server'/
mac address='52:54:00:3b:83:1a'/
model type='virtio'/
/interface

changes from v1:
 * addressed comments
 * removed unnecessary checks
 * series merged in a single patch

The previous version of this patch can be found at:
http://www.redhat.com/archives/libvir-list/2014-July/msg00111.html

Signed-off-by: Michele Paolino m.paol...@virtualopensystems.com
---
 docs/formatdomain.html.in  | 34 +
 docs/schemas/domaincommon.rng  | 25 +++
 src/conf/domain_conf.c | 87 ++
 src/conf/domain_conf.h | 10 ++-
 src/libxl/libxl_conf.c |  1 +
 src/lxc/lxc_process.c  |  1 +
 src/qemu/qemu_command.c| 63 
 src/uml/uml_conf.c |  5 ++
 src/xenxs/xen_sxpr.c   |  1 +
 .../qemuxml2argv-net-vhostuser.args|  7 ++
 .../qemuxml2argv-net-vhostuser.xml | 33 
 tests/qemuxml2argvtest.c   |  1 +
 tests/qemuxml2xmltest.c|  1 +
 13 files changed, 267 insertions(+), 2 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 3f8bbee..606b7d4 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3927,6 +3927,40 @@ qemu-kvm -net nic,model=? /dev/null
   span class=sinceSince 0.9.5/span
 /p
 
+h5a name=elementVhostuservhost-user interface/a/h5
+
+p
+ vhost-user enables the communication between a QEMU virtual machine
+ and other userspace process using the Virtio transport protocol.
+ A char dev (e.g. Unix socket) is used for the control plane, while
+ the data plane is based on shared memory.
+/p
+
+pre
+  ...
+  lt;devicesgt;
+lt;interface type='vhostuser'gt;
+  lt;source type='unix' path='/tmp/vhost.sock' mode='server'gt;
+  lt;/sourcegt;
+  lt;mac address='52:54:00:3b:83:1a'gt;
+  lt;/macgt;
+  lt;model type='virtio'gt;
+  lt;/modelgt;
+lt;/interfacegt;
+  lt;/devicesgt;
+  .../pre
+
+p
+  The codelt;sourcegt;/code element has to be specified
+  along with the type of char device.
+  Currently, only type='unix' is supported, where the path (the
+  directory path of the socket) and mode attributes are required.
+  Both codemode='server'/code and codemode='client'/code
+  are supported.
+  vhost-user requires the virtio model type, thus the
+  codelt;modelgt;/code element is mandatory.
+/p
+
 h4a name=elementsInputInput devices/a/h4
 
 p
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index af51eee..c9c02b6 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1968,6 +1968,31 @@
 /group
 group
   attribute name=type
+valuevhostuser/value
+  /attribute
+  interleave
+  element name=source
+attribute name=type
+  choice
+valueunix/value
+  /choice
+/attribute
+attribute name=path
+  ref name=absFilePath/
+/attribute
+attribute name=mode
+  choice
+valueserver/value
+valueclient/value
+  /choice
+/attribute
+empty/
+  /element
+ref name=interface-options/
+  /interleave
+/group
+group
+  attribute name=type
 valuenetwork/value
   /attribute
   interleave
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8df43b7..fb286c6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -349,6 +349,7 @@ VIR_ENUM_IMPL(virDomainFSWrpolicy, 
VIR_DOMAIN_FS_WRPOLICY_LAST,
 VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST,
   user,
   ethernet,
+  vhostuser,
   server,
   client,
   mcast,
@@ -1361,6 +1362,10 @@ void virDomainNetDefFree(virDomainNetDefPtr def)
 VIR_FREE(def-data.ethernet.ipaddr);
 break;
 
+case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+

Re: [libvirt] [PATCH 1/2] Don't output libvirt-UUID.files for LXC apparmor profiles

2014-07-11 Thread Cedric Bosdonnat
On Fri, 2014-07-11 at 11:03 -0600, Eric Blake wrote:
 On 07/11/2014 09:22 AM, Serge Hallyn wrote:
  Quoting Cédric Bosdonnat (cbosdon...@suse.com):
  ---
   src/security/virt-aa-helper.c | 3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)
  
  Hi,
  
  I'm acking this anyway bc I think you're right, but I'm trying to
  think of a case where this would still be useful.  What if we want
  to allow only a certain container to have access to its cgroups,
  for instance, for nesting containers.  Would virt-aa-helper and the
  .files be a way this would be done?  I suppose we coudl always re-introduce
  this in that case...  
  
  Acked-by: Serge E. Hallyn serge.hal...@ubuntu.com
 
 I've pushed this one.
 

Huh, I found a regression with this one... sent a v2 earlier today.

--
Cedric

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

Re: [libvirt] [PATCH 2/2] Rework lxc apparmor profile

2014-07-11 Thread Serge Hallyn
Quoting Cedric Bosdonnat (cbosdon...@suse.com):
 On Fri, 2014-07-11 at 16:08 +, Serge Hallyn wrote:
  Quoting Cédric Bosdonnat (cbosdon...@suse.com):
   Rework the apparmor lxc profile abstraction to mimic ubuntu's 
   container-default.
   This profile allows quite a lot, but strives to restrict access to
   dangerous resources.
   
   Removing the explicit authorizations to bash, systemd and cron files,
   forces them to keep the lxc profile for all applications inside the
   container. PUx permissions where leading to running systemd (and others
   tasks) unconfined.
   
   Put the generic files, network and capabilities restrictions directly
   in the TEMPLATE.lxc: this way, users can restrict them on a per
   container basis.
   ---
examples/apparmor/Makefile.am |   6 +-
examples/apparmor/TEMPLATE.lxc|  15 
examples/apparmor/{TEMPLATE = TEMPLATE.qemu} |   2 +-
examples/apparmor/libvirt-lxc | 119 
   +++---
src/security/security_apparmor.c  |  20 +++--
src/security/virt-aa-helper.c |  29 +--
6 files changed, 148 insertions(+), 43 deletions(-)
create mode 100644 examples/apparmor/TEMPLATE.lxc
rename examples/apparmor/{TEMPLATE = TEMPLATE.qemu} (75%)
   
   diff --git a/examples/apparmor/Makefile.am b/examples/apparmor/Makefile.am
   index a741e94..7a20e16 100644
   --- a/examples/apparmor/Makefile.am
   +++ b/examples/apparmor/Makefile.am
   @@ -15,7 +15,8 @@
## http://www.gnu.org/licenses/.

EXTRA_DIST=  \
   - TEMPLATE\
   + TEMPLATE.qemu   \
   + TEMPLATE.lxc\
 libvirt-qemu\
 libvirt-lxc \
 usr.lib.libvirt.virt-aa-helper  \
   @@ -36,6 +37,7 @@ abstractions_DATA = \

templatesdir = $(apparmordir)/libvirt
templates_DATA = \
   - TEMPLATE \
   + TEMPLATE.qemu \
   + TEMPLATE.lxc \
 $(NULL)
endif WITH_APPARMOR_PROFILES
   diff --git a/examples/apparmor/TEMPLATE.lxc 
   b/examples/apparmor/TEMPLATE.lxc
   new file mode 100644
   index 000..7b64885
   --- /dev/null
   +++ b/examples/apparmor/TEMPLATE.lxc
   @@ -0,0 +1,15 @@
   +#
   +# This profile is for the domain whose UUID matches this file.
   +#
   +
   +#include tunables/global
   +
   +profile LIBVIRT_TEMPLATE {
   +  #include abstractions/libvirt-lxc
   +
   +  # Globally allows everything to run under this profile
   +  # These can be narrowed depending on the container's use.
   +  file,
   +  capability,
   +  network,
   +}
   diff --git a/examples/apparmor/TEMPLATE b/examples/apparmor/TEMPLATE.qemu
   similarity index 75%
   rename from examples/apparmor/TEMPLATE
   rename to examples/apparmor/TEMPLATE.qemu
   index 187dec5..008a221 100644
   --- a/examples/apparmor/TEMPLATE
   +++ b/examples/apparmor/TEMPLATE.qemu
   @@ -5,5 +5,5 @@
#include tunables/global

profile LIBVIRT_TEMPLATE {
   -  #include abstractions/libvirt-driver
   +  #include abstractions/libvirt-qemu
}
   diff --git a/examples/apparmor/libvirt-lxc b/examples/apparmor/libvirt-lxc
   index d404328..4bfb503 100644
   --- a/examples/apparmor/libvirt-lxc
   +++ b/examples/apparmor/libvirt-lxc
   @@ -2,16 +2,115 @@

  #include abstractions/base

   -  # Needed for lxc-enter-namespace
   -  capability sys_admin,
   -  capability sys_chroot,
   +  umount,

   -  # Added for lxc-enter-namespace --cmd /bin/bash
   -  /bin/bash PUx,
   +  # ignore DENIED message on / remount
   +  deny mount options=(ro, remount) - /,

   -  /usr/sbin/cron PUx,
   -  /usr/lib/systemd/systemd PUx,
   +  # allow tmpfs mounts everywhere
   +  mount fstype=tmpfs,

   -  /usr/lib/libsystemd-*.so.* mr,
   -  /usr/lib/libudev-*.so.* mr,
   -  /etc/ld.so.cache mr,
   +  # allow mqueue mounts everywhere
   +  mount fstype=mqueue,
   +
   +  # allow fuse mounts everywhere
   +  mount fstype=fuse.*,
   +
   +  # deny writes in /proc/sys/fs but allow binfmt_misc to be mounted
   +  mount fstype=binfmt_misc - /proc/sys/fs/binfmt_misc/,
   +  deny @{PROC}/sys/fs/** wklx,
   +
   +  # allow efivars to be mounted, writing to it will be blocked though
   +  mount fstype=efivarfs - /sys/firmware/efi/efivars/,
   +
   +  # block some other dangerous paths
   +  deny @{PROC}/sysrq-trigger rwklx,
   +  deny @{PROC}/mem rwklx,
   +  deny @{PROC}/kmem rwklx,
   +
   +  # deny writes in /sys except for /sys/fs/cgroup, also allow
   +  # fusectl, securityfs and debugfs to be mounted there (read-only)
   +  mount fstype=fusectl - /sys/fs/fuse/connections/,
   +  mount fstype=securityfs - /sys/kernel/security/,
   +  mount fstype=debugfs - /sys/kernel/debug/,
   +  mount fstype=proc - /proc/,
   +  mount fstype=sysfs - /sys/,
   +  deny /sys/firmware/efi/efivars/** rwklx,
   +  deny /sys/kernel/security/** rwklx,
   +
   +  # generated by: lxc-generate-aa-rules.py 

Re: [libvirt] [PATCH 2/2] Rework lxc apparmor profile

2014-07-11 Thread Cedric Bosdonnat
On Fri, 2014-07-11 at 16:08 +, Serge Hallyn wrote:
 Quoting Cédric Bosdonnat (cbosdon...@suse.com):
  Rework the apparmor lxc profile abstraction to mimic ubuntu's 
  container-default.
  This profile allows quite a lot, but strives to restrict access to
  dangerous resources.
  
  Removing the explicit authorizations to bash, systemd and cron files,
  forces them to keep the lxc profile for all applications inside the
  container. PUx permissions where leading to running systemd (and others
  tasks) unconfined.
  
  Put the generic files, network and capabilities restrictions directly
  in the TEMPLATE.lxc: this way, users can restrict them on a per
  container basis.
  ---
   examples/apparmor/Makefile.am |   6 +-
   examples/apparmor/TEMPLATE.lxc|  15 
   examples/apparmor/{TEMPLATE = TEMPLATE.qemu} |   2 +-
   examples/apparmor/libvirt-lxc | 119 
  +++---
   src/security/security_apparmor.c  |  20 +++--
   src/security/virt-aa-helper.c |  29 +--
   6 files changed, 148 insertions(+), 43 deletions(-)
   create mode 100644 examples/apparmor/TEMPLATE.lxc
   rename examples/apparmor/{TEMPLATE = TEMPLATE.qemu} (75%)
  
  diff --git a/examples/apparmor/Makefile.am b/examples/apparmor/Makefile.am
  index a741e94..7a20e16 100644
  --- a/examples/apparmor/Makefile.am
  +++ b/examples/apparmor/Makefile.am
  @@ -15,7 +15,8 @@
   ## http://www.gnu.org/licenses/.
   
   EXTRA_DIST=\
  -   TEMPLATE\
  +   TEMPLATE.qemu   \
  +   TEMPLATE.lxc\
  libvirt-qemu\
  libvirt-lxc \
  usr.lib.libvirt.virt-aa-helper  \
  @@ -36,6 +37,7 @@ abstractions_DATA = \
   
   templatesdir = $(apparmordir)/libvirt
   templates_DATA = \
  -   TEMPLATE \
  +   TEMPLATE.qemu \
  +   TEMPLATE.lxc \
  $(NULL)
   endif WITH_APPARMOR_PROFILES
  diff --git a/examples/apparmor/TEMPLATE.lxc b/examples/apparmor/TEMPLATE.lxc
  new file mode 100644
  index 000..7b64885
  --- /dev/null
  +++ b/examples/apparmor/TEMPLATE.lxc
  @@ -0,0 +1,15 @@
  +#
  +# This profile is for the domain whose UUID matches this file.
  +#
  +
  +#include tunables/global
  +
  +profile LIBVIRT_TEMPLATE {
  +  #include abstractions/libvirt-lxc
  +
  +  # Globally allows everything to run under this profile
  +  # These can be narrowed depending on the container's use.
  +  file,
  +  capability,
  +  network,
  +}
  diff --git a/examples/apparmor/TEMPLATE b/examples/apparmor/TEMPLATE.qemu
  similarity index 75%
  rename from examples/apparmor/TEMPLATE
  rename to examples/apparmor/TEMPLATE.qemu
  index 187dec5..008a221 100644
  --- a/examples/apparmor/TEMPLATE
  +++ b/examples/apparmor/TEMPLATE.qemu
  @@ -5,5 +5,5 @@
   #include tunables/global
   
   profile LIBVIRT_TEMPLATE {
  -  #include abstractions/libvirt-driver
  +  #include abstractions/libvirt-qemu
   }
  diff --git a/examples/apparmor/libvirt-lxc b/examples/apparmor/libvirt-lxc
  index d404328..4bfb503 100644
  --- a/examples/apparmor/libvirt-lxc
  +++ b/examples/apparmor/libvirt-lxc
  @@ -2,16 +2,115 @@
   
 #include abstractions/base
   
  -  # Needed for lxc-enter-namespace
  -  capability sys_admin,
  -  capability sys_chroot,
  +  umount,
   
  -  # Added for lxc-enter-namespace --cmd /bin/bash
  -  /bin/bash PUx,
  +  # ignore DENIED message on / remount
  +  deny mount options=(ro, remount) - /,
   
  -  /usr/sbin/cron PUx,
  -  /usr/lib/systemd/systemd PUx,
  +  # allow tmpfs mounts everywhere
  +  mount fstype=tmpfs,
   
  -  /usr/lib/libsystemd-*.so.* mr,
  -  /usr/lib/libudev-*.so.* mr,
  -  /etc/ld.so.cache mr,
  +  # allow mqueue mounts everywhere
  +  mount fstype=mqueue,
  +
  +  # allow fuse mounts everywhere
  +  mount fstype=fuse.*,
  +
  +  # deny writes in /proc/sys/fs but allow binfmt_misc to be mounted
  +  mount fstype=binfmt_misc - /proc/sys/fs/binfmt_misc/,
  +  deny @{PROC}/sys/fs/** wklx,
  +
  +  # allow efivars to be mounted, writing to it will be blocked though
  +  mount fstype=efivarfs - /sys/firmware/efi/efivars/,
  +
  +  # block some other dangerous paths
  +  deny @{PROC}/sysrq-trigger rwklx,
  +  deny @{PROC}/mem rwklx,
  +  deny @{PROC}/kmem rwklx,
  +
  +  # deny writes in /sys except for /sys/fs/cgroup, also allow
  +  # fusectl, securityfs and debugfs to be mounted there (read-only)
  +  mount fstype=fusectl - /sys/fs/fuse/connections/,
  +  mount fstype=securityfs - /sys/kernel/security/,
  +  mount fstype=debugfs - /sys/kernel/debug/,
  +  mount fstype=proc - /proc/,
  +  mount fstype=sysfs - /sys/,
  +  deny /sys/firmware/efi/efivars/** rwklx,
  +  deny /sys/kernel/security/** rwklx,
  +
  +  # generated by: lxc-generate-aa-rules.py container-rules.base
  +  deny /proc/sys/[^kn]*{,/**} wklx,
  +  deny /proc/sys/k[^e]*{,/**} wklx,
  +  deny /proc/sys/ke[^r]*{,/**} wklx,
  +  deny /proc/sys/ker[^n]*{,/**} wklx,

Re: [libvirt] [PREPOST 07/17] src/xenxs:Refactor disk config parsing code

2014-07-11 Thread Jim Fehlig
David Kiarie wrote:
 From: Kiarie Kahurani davidkiar...@gmail.com

 Introduce the function
  xenParseXMDisk(..);

 Parses disk config

 signed-off-by: David Kiariedavidkiar...@gmail.com
 ---
  src/xenxs/xen_xm.c | 204 
 -
  1 file changed, 108 insertions(+), 96 deletions(-)

 diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
 index ebeeeb5..80a7941 100644
 --- a/src/xenxs/xen_xm.c
 +++ b/src/xenxs/xen_xm.c
 @@ -685,87 +685,14 @@ static int xenParseXMCPUFeatures(virConfPtr conf, 
 virDomainDefPtr def)
  
  return 0;
  }
 -virDomainDefPtr
 -xenParseXM(virConfPtr conf, int xendConfigVersion,
 -   virCapsPtr caps)
 +static int xenParseXMDisk(virConfPtr conf, virDomainDefPtr def,
 +   int xendConfigVersion)
  {
 -const char *str;
 -int hvm = 0;
 -int val;
 -virConfValuePtr list;
 -virDomainDefPtr def = NULL;
 +const char *str = NULL;
  virDomainDiskDefPtr disk = NULL;
 -virDomainNetDefPtr net = NULL;
 -virDomainGraphicsDefPtr graphics = NULL;
 -size_t i;
 -char *script = NULL;
 -char *listenAddr = NULL;
 -
 -if (VIR_ALLOC(def)  0)
 -return NULL;
 -
 -def-virtType = VIR_DOMAIN_VIRT_XEN;
 -def-id = -1;
 -if (xenParseXMGeneral(conf, def, caps)  0)
 -goto cleanup;
 -hvm = (STREQ(def-os.type, hvm));
 -if (hvm) {
 -const char *boot;
 -if (xenXMConfigCopyString(conf, kernel, def-os.loader)  0)
 -goto cleanup;
 -
 -if (xenXMConfigGetString(conf, boot, boot, c)  0)
 -goto cleanup;
 +int hvm = STREQ(def-os.type, hvm);
 +virConfValuePtr list = virConfGetValue(conf, disk);
  
 -for (i = 0; i  VIR_DOMAIN_BOOT_LAST  boot[i]; i++) {
 -switch (*boot) {
 -case 'a':
 -def-os.bootDevs[i] = VIR_DOMAIN_BOOT_FLOPPY;
 -break;
 -case 'd':
 -def-os.bootDevs[i] = VIR_DOMAIN_BOOT_CDROM;
 -break;
 -case 'n':
 -def-os.bootDevs[i] = VIR_DOMAIN_BOOT_NET;
 -break;
 -case 'c':
 -default:
 -def-os.bootDevs[i] = VIR_DOMAIN_BOOT_DISK;
 -break;
 -}
 -def-os.nBootDevs++;
 -}
 -} else {
 -const char *extra, *root;
 -
 -if (xenXMConfigCopyStringOpt(conf, bootloader, 
 def-os.bootloader)  0)
 -goto cleanup;
 -if (xenXMConfigCopyStringOpt(conf, bootargs, 
 def-os.bootloaderArgs)  0)
 -goto cleanup;
 -
 -if (xenXMConfigCopyStringOpt(conf, kernel, def-os.kernel)  0)
 -goto cleanup;
 -if (xenXMConfigCopyStringOpt(conf, ramdisk, def-os.initrd)  0)
 -goto cleanup;
 -if (xenXMConfigGetString(conf, extra, extra, NULL)  0)
 -goto cleanup;
 -if (xenXMConfigGetString(conf, root, root, NULL)  0)
 -goto cleanup;
 -
 -if (root) {
 -if (virAsprintf(def-os.cmdline, root=%s %s, root, extra)  0)
 -goto cleanup;
 -} else {
 -if (VIR_STRDUP(def-os.cmdline, extra)  0)
 -goto cleanup;
 -}
 -}
 -if (xenParseXMMem(conf, def)  0)
 -goto cleanup;
 -if (xenXMConfigCopyStringOpt(conf, device_model, def-emulator)  0)
 -goto cleanup;
 -
 -list = virConfGetValue(conf, disk);
  if (list  list-type == VIR_CONF_LIST) {
  list = list-list;
  while (list) {
 @@ -779,7 +706,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  head = list-str;
  
  if (!(disk = virDomainDiskDefNew()))
 -goto cleanup;
 +return -1;
  
  /*
   * Disks have 3 components, SOURCE,DEST-DEVICE,MODE
 @@ -798,10 +725,10 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  ignore_value(virDomainDiskSetSource(disk, NULL));
  } else {
  if (VIR_STRNDUP(tmp, head, offset - head)  0)
 -goto cleanup;
 +return -1;
  if (virDomainDiskSetSource(disk, tmp)  0) {
  VIR_FREE(tmp);
 -goto cleanup;
 +return -1;
  }
  VIR_FREE(tmp);
  }
 @@ -815,12 +742,12 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  if (!(offset = strchr(head, ',')))
  goto skipdisk;
  if (VIR_ALLOC_N(disk-dst, (offset - head) + 1)  0)
 -goto cleanup;
 +return -1;
  if (virStrncpy(disk-dst, head, offset - head,
 (offset - head) + 1) == NULL) {
  virReportError(VIR_ERR_INTERNAL_ERROR,
 _(Dest file %s too big for destination), 
 head);
 -goto cleanup;
 +

Re: [libvirt] [PREPOST 07/17] src/xenxs:Refactor disk config parsing code

2014-07-11 Thread Eric Blake
On 07/11/2014 01:04 PM, Jim Fehlig wrote:
 David Kiarie wrote:
 From: Kiarie Kahurani davidkiar...@gmail.com

 Introduce the function
  xenParseXMDisk(..);

 Parses disk config

 signed-off-by: David Kiariedavidkiar...@gmail.com
 ---
  src/xenxs/xen_xm.c | 204 
 -
  1 file changed, 108 insertions(+), 96 deletions(-)


 @@ -912,7 +839,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  
  /* Maintain list in sorted order according to target device 
 name */
  if (VIR_APPEND_ELEMENT(def-disks, def-ndisks, disk)  0)
 -goto cleanup;
 +return -1;
  
  skipdisk:
  list = list-next;
   
 
 The next line here is
 
 virDomainDiskDefFree(disk);
 
 which is not right in the case VIR_APPEND_ELEMENT succeeds (pre-existing
 bug).  'disk' should be set to NULL if VIR_APPEND_ELEMENT succeeds, so
 we don't free the disk we've just added to the disk list.

VIR_APPEND_ELEMENT(, , disk) sets disk=NULL on success, precisely so you
don't have to worry about double-freeing it (if you call the macro, you
are guaranteed either transfer semantics and success, or no change on
failure).


-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCHv2 1/2] Don't output libvirt-UUID.files for LXC apparmor profiles

2014-07-11 Thread Eric Blake
On 07/11/2014 07:01 AM, Cédric Bosdonnat wrote:
 ---
  src/security/virt-aa-helper.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)
 
 diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
 index b5f66f3..c8f17f9 100644
 --- a/src/security/virt-aa-helper.c
 +++ b/src/security/virt-aa-helper.c
 @@ -1342,10 +1342,13 @@ main(int argc, char **argv)
  vah_info(include_file);
  vah_info(included_files);
  rc = 0;
 +} else if (ctl-def-virtType == VIR_DOMAIN_VIRT_LXC) {
 +rc = 0;
  } else if ((rc = update_include_file(include_file,
   included_files,
 - ctl-append)) != 0)
 + ctl-append)) != 0) {
  goto cleanup;
 +}

I squashed this on top of a revert of v1, since I had pushed that before
realizing you had posted a v2, and pushed the result.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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] virseclabel: Resolve Coverity FORWARD_NULL issue

2014-07-11 Thread Eric Blake
On 07/11/2014 07:42 AM, John Ferlan wrote:
 Resolve issue introduced by commit id '13adf1b'
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/util/virseclabel.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

ACK

 
 diff --git a/src/util/virseclabel.c b/src/util/virseclabel.c
 index e9d973c..02a8342 100644
 --- a/src/util/virseclabel.c
 +++ b/src/util/virseclabel.c
 @@ -61,7 +61,7 @@ virSecurityLabelDefNew(const char *model)
  if (VIR_ALLOC(seclabel)  0 ||
  VIR_STRDUP(seclabel-model, model)  0) {
  virSecurityLabelDefFree(seclabel);
 -seclabel = NULL;
 +return NULL;
  }
  
  seclabel-relabel = true;
 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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] conf: Fix possible NULL dereference in virStorageVolTargetDefFormat

2014-07-11 Thread Eric Blake
On 07/11/2014 11:00 AM, Eric Blake wrote:
 On 07/11/2014 10:13 AM, Matthias Bolte wrote:
 Commit dae1568c6c6455091e8cd9bc2e90a22af3d3880c converted the perms
 member of the virStorageVolTarget struct into a pointer to make it
 optional. But virStorageVolTargetDefFormat did not check perms for
 NULL before dereferencing it.
 ---
  src/conf/storage_conf.c | 26 ++
  1 file changed, 14 insertions(+), 12 deletions(-)
 
 ACK.
 

Now pushed.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

[libvirt] [PATCHv2]util:openvswitch:Delete port if it is exist when add port

2014-07-11 Thread Lichunhe
If the ovs service stop abnormal, or host cold reboot, vm is destroyed after 
ovs service stop. The ovs port which connect to interface of vm will not be 
clear. When the ovs service restart, recover configuration from db, but the 
interface is no exist, port recovery failed, and then vm restart on the same 
host, libvirt add port again, but the port configuration is same as before, ovs 
will not connect the interface, only store the configuration in db. Below will 
trigger this problem,

virsh start vm
service openvswitch-switch stop
virsh destroy vm
service openvswitch-switch start
virsh start vm

Signed-off-by: Chunhe Li lichu...@huawei.com
---

src/util/virnetdevopenvswitch.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
index 9bcbfb1..2c414ad 100644
--- a/src/util/virnetdevopenvswitch.c
+++ b/src/util/virnetdevopenvswitch.c
@@ -84,8 +84,8 @@ int virNetDevOpenvswitchAddPort(const char *brname, const 
char *ifname,
 
 cmd = virCommandNew(OVSVSCTL);
 
-virCommandAddArgList(cmd, --timeout=5, --, --may-exist, add-port,
-brname, ifname, NULL);
+virCommandAddArgList(cmd, --timeout=5, --, --if-exists, del-port, 
+ifname, --, add-port, brname, ifname, NULL);
 
 if (virtVlan  virtVlan-nTags  0) {
--
1.9.2.msysgit.0

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


Re: [libvirt] [PREPOST 07/17] src/xenxs:Refactor disk config parsing code

2014-07-11 Thread Jim Fehlig
Eric Blake wrote:
 On 07/11/2014 01:04 PM, Jim Fehlig wrote:
   
 David Kiarie wrote:
 
 From: Kiarie Kahurani davidkiar...@gmail.com

 Introduce the function
  xenParseXMDisk(..);

 Parses disk config

 signed-off-by: David Kiariedavidkiar...@gmail.com
 ---
  src/xenxs/xen_xm.c | 204 
 -
  1 file changed, 108 insertions(+), 96 deletions(-)

   

   
 @@ -912,7 +839,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  
  /* Maintain list in sorted order according to target device 
 name */
  if (VIR_APPEND_ELEMENT(def-disks, def-ndisks, disk)  0)
 -goto cleanup;
 +return -1;
  
  skipdisk:
  list = list-next;
   
   
 The next line here is

 virDomainDiskDefFree(disk);

 which is not right in the case VIR_APPEND_ELEMENT succeeds (pre-existing
 bug).  'disk' should be set to NULL if VIR_APPEND_ELEMENT succeeds, so
 we don't free the disk we've just added to the disk list.
 

 VIR_APPEND_ELEMENT(, , disk) sets disk=NULL on success, precisely so you
 don't have to worry about double-freeing it (if you call the macro, you
 are guaranteed either transfer semantics and success, or no change on
 failure).
   

Ah, should have read the source :).  Thanks Eric.  That explains my
confusion as to why a crash was never reported in this code (or the vif
parsing code).

Regards,
Jim

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