[libvirt] [PATCH 2/6] unpriv_sgio: Parse and format the new XML

[Osier Yang]

Setting unpriv_sgio to yes to enable the unprivleged SG_IO,
and no to disable it. Later patch will do the actual setting.
---
 src/conf/domain_conf.c |   33 ++-
 src/conf/domain_conf.h |   15 -
 ...l2argv-disk-scsi-lun-passthrough-upriv-sgio.xml |   32 +++
 tests/qemuxml2xmltest.c|1 +
 4 files changed, 57 insertions(+), 24 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough-upriv-sgio.xml

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e57dbd0..b25229a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -236,7 +236,7 @@ VIR_ENUM_IMPL(virDomainDiskIo, VIR_DOMAIN_DISK_IO_LAST,
   default,
   native,
   threads)
-VIR_ENUM_IMPL(virDomainDiskSGIO, VIR_DOMAIN_DISK_SGIO_LAST,
+VIR_ENUM_IMPL(virDomainDiskUnprivSGIO, VIR_DOMAIN_DISK_UNPRIV_SGIO_LAST,
   default,
   yes,
   no)
@@ -3519,7 +3519,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
 char *device = NULL;
 char *snapshot = NULL;
 char *rawio = NULL;
-char *sgio = NULL;
+char *unpriv_sgio = NULL;
 char *driverName = NULL;
 char *driverType = NULL;
 char *source = NULL;
@@ -3581,7 +3581,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
 snapshot = virXMLPropString(node, snapshot);
 
 rawio = virXMLPropString(node, rawio);
-sgio = virXMLPropString(node, sgio);
+unpriv_sgio = virXMLPropString(node, unpriv_sgio);
 
 cur = node-children;
 while (cur != NULL) {
@@ -3972,13 +3972,13 @@ virDomainDiskDefParseXML(virCapsPtr caps,
 def-snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE;
 }
 
-if (rawio  sgio) {
+if (rawio  unpriv_sgio) {
 virReportError(VIR_ERR_XML_ERROR, %s,
-   _(rawio and sgio are exclusive));
+   _(rawio and unpriv_sgio are exclusive));
 goto error;
 }
 
-if ((rawio || sgio) 
+if ((rawio || unpriv_sgio) 
 (def-device != VIR_DOMAIN_DISK_DEVICE_LUN)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(rawio can be used only with device='lun'));
@@ -3999,17 +3999,18 @@ virDomainDiskDefParseXML(virCapsPtr caps,
 }
 }
 
-if (sgio) {
-int sgioVal = 0;
+if (unpriv_sgio) {
+int unpriv_sgio_val = 0;
 
-if ((sgioVal = virDomainDiskSGIOTypeFromString(sgio))  0) {
+if ((unpriv_sgio_val =
+ virDomainDiskUnprivSGIOTypeFromString(unpriv_sgio))  0) {
 virReportError(VIR_ERR_XML_ERROR,
-   _(unknown disk sgio setting '%s'),
-   sgio);
+   _(unknown disk unpriv_sgio setting '%s'),
+   unpriv_sgio);
 goto error;
 }
 
-def-sgio = sgioVal;
+def-unpriv_sgio = unpriv_sgio_val;
 }
 
 if (bus) {
@@ -4246,7 +4247,7 @@ cleanup:
 VIR_FREE(type);
 VIR_FREE(snapshot);
 VIR_FREE(rawio);
-VIR_FREE(sgio);
+VIR_FREE(unpriv_sgio);
 VIR_FREE(target);
 VIR_FREE(source);
 VIR_FREE(tray);
@@ -11924,9 +11925,9 @@ virDomainDiskDefFormat(virBufferPtr buf,
 virBufferAddLit(buf,  rawio='no');
 }
 }
-if (def-sgio)
-virBufferAsprintf(buf,sgio='%s',
-  virDomainDiskSGIOTypeToString(def-sgio));
+if (def-unpriv_sgio)
+virBufferAsprintf(buf,  unpriv_sgio='%s',
+  
virDomainDiskUnprivSGIOTypeToString(def-unpriv_sgio));
 if (def-snapshot 
 !(def-snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE  def-readonly))
 virBufferAsprintf(buf,  snapshot='%s',
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 9e1a9bb..105fb7d 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -496,12 +496,12 @@ enum  virDomainDiskIo {
 VIR_DOMAIN_DISK_IO_LAST
 };
 
-enum virDomainDiskSGIO {
-VIR_DOMAIN_DISK_SGIO_DEFAULT = 0,
-VIR_DOMAIN_DISK_SGIO_YES,
-VIR_DOMAIN_DISK_SGIO_NO,
+enum virDomainDiskUnprivSGIO {
+VIR_DOMAIN_DISK_UNPRIV_SGIO_DEFAULT = 0,
+VIR_DOMAIN_DISK_UNPRIV_SGIO_YES,
+VIR_DOMAIN_DISK_UNPRIV_SGIO_NO,
 
-VIR_DOMAIN_DISK_SGIO_LAST
+VIR_DOMAIN_DISK_UNPRIV_SGIO_LAST
 };
 
 enum virDomainIoEventFd {
@@ -615,8 +615,7 @@ struct _virDomainDiskDef {
 virStorageEncryptionPtr encryption;
 bool rawio_specified;
 int rawio; /* no = 0, yes = 1 */
-int sgio;  /* no = 0, yes = 1 */
-int old_sgio; /* To record the old unpriv_sgio value, internally */
+int unpriv_sgio;  /* no = 0, yes = 1 */
 
 size_t nseclabels;
 virSecurityDeviceLabelDefPtr *seclabels;
@@ -2207,7 +2206,7 @@ VIR_ENUM_DECL(virDomainDiskCache)
 VIR_ENUM_DECL(virDomainDiskErrorPolicy)
 VIR_ENUM_DECL(virDomainDiskProtocol)
 VIR_ENUM_DECL(virDomainDiskIo)

[libvirt] [PATCH 1/6] unpriv_sgio: Add docs and rng schema for new XML unpriv_sgio

[Osier Yang]

Since rawio and unpriv_sgio are only valid for lun, this
groups them together. And since both of them intend to allow
the unprivledged user to use the SCSI commands, they are must be
exclusive. Actually unpriv_sgio supersedes rawio, as it
confines the capability per-device, unlike rawio, which gives
the domain process broad capablity.
---
 docs/formatdomain.html.in |   10 +++-
 docs/schemas/domaincommon.rng |   52 ++
 src/conf/domain_conf.c|   56 
 src/conf/domain_conf.h|   11 +++
 src/libvirt_private.syms  |4 +
 src/qemu/qemu_process.c   |   30 
 src/util/util.c   |  153 +
 src/util/util.h   |7 ++
 8 files changed, 293 insertions(+), 30 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 6a3b976..f3f6a9e 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1395,7 +1395,15 @@
 rawio='yes', rawio capability will be enabled for all disks in
 the domain (because, in the case of QEMU, this capability can
 only be set on a per-process basis). This attribute is only
-valid when device is lun.
+valid when device is lun. NB, coderawio/code gives
+the domain process broad capability, to confined the capability
+as much as possible, one should use codeunpriv_sgio/code
+instead, which controls the capability per-device.
+The optional codeunpriv_sgio/code attribute
+(span class=sincesince 1.0.1/span) indicates whether the
+disk will allow unprivileged SG_IO, valid settings are yes
+or no (defaults to no). Note that it's exclusive with
+attribute coderawio/code;
 The optional codesnapshot/code attribute indicates the default
 behavior of the disk during disk snapshots: internal
 requires a file format such as qcow2 that can store both the
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 02ad477..7da571c 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -957,24 +957,44 @@
 --
   define name=disk
 element name=disk
-  optional
-attribute name=device
-  choice
-valuefloppy/value
-valuedisk/value
-valuecdrom/value
-valuelun/value
-  /choice
-/attribute
-  /optional
-  optional
-attribute name=rawio
+  choice
+group
+  optional
+attribute name=device
+  choice
+valuefloppy/value
+valuedisk/value
+valuecdrom/value
+  /choice
+/attribute
+  /optional
+/group
+group
+  optional
+attribute name=device
+  valuelun/value
+/attribute
+  /optional
   choice
-valueyes/value
-valueno/value
+optional
+  attribute name=rawio
+choice
+  valueyes/value
+  valueno/value
+/choice
+  /attribute
+/optional
+optional
+  attribute name=unpriv_sgio
+choice
+  valueyes/value
+  valueno/value
+/choice
+  /attribute
+/optional
   /choice
-/attribute
-  /optional
+/group
+  /choice
   optional
 ref name=snapshot/
   /optional
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ed8b53f..e57dbd0 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -236,6 +236,10 @@ VIR_ENUM_IMPL(virDomainDiskIo, VIR_DOMAIN_DISK_IO_LAST,
   default,
   native,
   threads)
+VIR_ENUM_IMPL(virDomainDiskSGIO, VIR_DOMAIN_DISK_SGIO_LAST,
+  default,
+  yes,
+  no)
 VIR_ENUM_IMPL(virDomainIoEventFd, VIR_DOMAIN_IO_EVENT_FD_LAST,
   default,
   on,
@@ -3515,6 +3519,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
 char *device = NULL;
 char *snapshot = NULL;
 char *rawio = NULL;
+char *sgio = NULL;
 char *driverName = NULL;
 char *driverType = NULL;
 char *source = NULL;
@@ -3576,6 +3581,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
 snapshot = virXMLPropString(node, snapshot);
 
 rawio = virXMLPropString(node, rawio);
+sgio = virXMLPropString(node, sgio);
 
 cur = node-children;
 while (cur != NULL) {
@@ -3966,24 +3972,44 @@ virDomainDiskDefParseXML(virCapsPtr caps,
 def-snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE;
 }
 
+if (rawio  sgio) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(rawio and sgio are exclusive));
+goto error;
+}
+
+if ((rawio || sgio) 
+(def-device 

[libvirt] [PATCH 0/6] Unprivileged SG_IO support

[Osier Yang]

Hi,

As a result of RFC [1], this implements the unprivleged SG_IO
support. Though this is a bit rushed, and I still have no chance
to test it yet, but I'd like see an earlier reviewing.

@Paolo, please let me known once you have a kernel build with
the new sysfs knob support. Thanks!

Osier Yang (6):
  unpriv_sgio: Add docs and rng schema for new XML unpriv_sgio
  unpriv_sgio: Parse and format the new XML
  unpriv_sgio: Prepare helpers for unpriv_sgio setting
  unpriv_sgio: Manage unpriv_sgio in domain's lifecyle
  unpriv_sgio: Do not restore unpriv_sgio if the disk is being used
  unpriv_sgio: Error out if the unpriv_sgio setting conflicts with
others

 docs/formatdomain.html.in  |   10 +-
 docs/schemas/domaincommon.rng  |   52 --
 src/conf/domain_conf.c |  168 ++--
 src/conf/domain_conf.h |   18 ++
 src/libvirt_private.syms   |6 +
 src/qemu/qemu_process.c|   54 +++
 src/util/util.c|  154 ++
 src/util/util.h|7 +
 ...l2argv-disk-scsi-lun-passthrough-upriv-sgio.xml |   32 
 tests/qemuxml2xmltest.c|1 +
 10 files changed, 472 insertions(+), 30 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough-upriv-sgio.xml

[1] https://www.redhat.com/archives/libvir-list/2012-November/msg00988.html

Regards,
Osier

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

[libvirt] [PATCH 6/6] unpriv_sgio: Error out if the unpriv_sgio setting conflicts with others

[Osier Yang]

This prevent the domain starting if the shared disk's setting conflicts
with other active domain(s), E.g. A domain with unpriv_sgio set as
yes, however, another active domain is using it set as no.
---
 src/conf/domain_conf.c   |   45 +
 src/conf/domain_conf.h   |3 +++
 src/libvirt_private.syms |1 +
 src/qemu/qemu_process.c  |   14 +-
 4 files changed, 62 insertions(+), 1 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 02df96e..fb7bb1f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15763,3 +15763,48 @@ virDomainDiskIsUsed(const virDomainObjList doms,
 
 return false;
 }
+
+static int
+virDomainSharedDiskCompare(const void *payload,
+   const void *name ATTRIBUTE_UNUSED,
+   const void *opaque)
+{
+virDomainObjPtr obj = (virDomainObjPtr)payload;
+const virDomainDiskDefPtr disk = (virDomainDiskDefPtr)opaque;
+int i;
+
+virDomainObjLock(obj);
+
+/* Ingores the inactive ones */
+if (!virDomainObjIsActive(obj))
+return 0;
+
+for (i = 0; i  obj-def-ndisks; i++) {
+if (STRNEQ(obj-def-disks[i]-src, disk-src))
+continue;
+
+if (obj-def-disks[i]-unpriv_sgio != disk-unpriv_sgio) {
+virDomainObjUnlock(obj);
+return 1;
+}
+}
+
+virDomainObjUnlock(obj);
+return 0;
+}
+
+/* Validate if the shared disk conf is conflicted with other domains,
+ * currently only validates the unpriv_sgio setting
+ */
+bool
+virDomainSharedDiskValidate(const virDomainObjList doms,
+const virDomainDiskDefPtr disk)
+{
+virDomainObjPtr obj;
+
+obj = virHashSearch(doms.objs, virDomainSharedDiskCompare, disk);
+if (obj)
+return false;
+
+return true;
+}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3105e05..35f46d2 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1878,6 +1878,9 @@ bool virDomainDiskIsUsed(const virDomainObjList doms,
  char *name,
  char *diskSrc,
  bool active);
+bool
+virDomainSharedDiskValidate(const virDomainObjList doms,
+const virDomainDiskDefPtr disk);
 
 bool virDomainObjTaint(virDomainObjPtr obj,
enum virDomainTaintFlags taint);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b9019b7..b14eb40 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -489,6 +489,7 @@ virDomainSaveStatus;
 virDomainSaveXML;
 virDomainSeclabelTypeFromString;
 virDomainSeclabelTypeToString;
+virDomainSharedDiskValidate;
 virDomainShutdownReasonTypeFromString;
 virDomainShutdownReasonTypeToString;
 virDomainShutoffReasonTypeFromString;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index eeaaea0..b5676e0 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3711,6 +3711,18 @@ int qemuProcessStart(virConnectPtr conn,
 if (!disk-unpriv_sgio)
 continue;
 
+ /* Error out if the disk is shared, but the unpriv_sgio conflicts
+  * with other active domain(s).
+  */
+if (disk-shared 
+!virDomainSharedDiskValidate(driver-domains,
+ disk)) {
+virReportError(VIR_ERR_XML_ERROR,
+   _(Setting of shared disk '%s' conflicts with 
+ other active domains), disk-src);
+goto cleanup;
+}
+
 if (virGetDeviceUnprivSGIO(disk-src, old_unpriv_sgio)  0)
 goto cleanup;
 
@@ -3719,7 +3731,7 @@ int qemuProcessStart(virConnectPtr conn,
 if (virSetDeviceUnprivSGIO(disk-src,
(disk-unpriv_sgio ==
 VIR_DOMAIN_DISK_UNPRIV_SGIO_YES)
-   ? 1 : 0)  0)
+? 1 : 0)  0)
 goto cleanup;
 }
 
-- 
1.7.7.6

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

[libvirt] [PATCH 4/6] unpriv_sgio: Manage unpriv_sgio in domain's lifecyle

[Osier Yang]

To record the original unpriv_sgio value, this introduces
old_unpriv_sgio for disk def. When the domain is starting,
the disk's unpriv_sgio is set with regards to the config
in domain XML. And when the domain is being destroyed, it's
restored to the original value (old_unpriv_sgio).
---
 src/conf/domain_conf.h  |1 +
 src/qemu/qemu_process.c |   19 ++-
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 105fb7d..1a8de71 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -616,6 +616,7 @@ struct _virDomainDiskDef {
 bool rawio_specified;
 int rawio; /* no = 0, yes = 1 */
 int unpriv_sgio;  /* no = 0, yes = 1 */
+int old_unpriv_sgio; /* To record the old unpriv_sgio value, internally */
 
 size_t nseclabels;
 virSecurityDeviceLabelDefPtr *seclabels;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 98096c5..e48eed0 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3706,19 +3706,20 @@ int qemuProcessStart(virConnectPtr conn,
 /* Set unpriv_sgio for disks */
 for (i = 0; i  vm-def-ndisks; i++) {
 virDomainDiskDefPtr disk = vm-def-disks[i];
-int old_sgio;
+int old_unpriv_sgio;
 
-if (!disk-sgio)
+if (!disk-unpriv_sgio)
 continue;
 
-if (virGetDeviceSGIO(disk-src, old_sgio)  0)
+if (virGetDeviceUnprivSGIO(disk-src, old_unpriv_sgio)  0)
 goto cleanup;
 
-disk-old_sgio = old_sgio;
+disk-old_unpriv_sgio = old_unpriv_sgio;
 
-if (virSetDeviceSGIO(disk-src,
- (disk-sgio == VIR_DOMAIN_DISK_SGIO_YES)
- ? 1 : 0)  0)
+if (virSetDeviceUnprivSGIO(disk-src,
+   (disk-unpriv_sgio ==
+VIR_DOMAIN_DISK_UNPRIV_SGIO_YES)
+   ? 1 : 0)  0)
 goto cleanup;
 }
 
@@ -4116,10 +4117,10 @@ void qemuProcessStop(struct qemud_driver *driver,
 for (i = 0; i  vm-def-ndisks; i++) {
 virDomainDiskDefPtr disk = vm-def-disks[i];
 
-if (!disk-sgio)
+if (!disk-unpriv_sgio)
 continue;
 
-if (virSetDeviceSGIO(disk-src, disk-old_sgio)  0)
+if (virSetDeviceUnprivSGIO(disk-src, disk-old_unpriv_sgio)  0)
 VIR_WARN(Unable to restore unpriv_sgio for disk '%s', disk-src);
 }
 
-- 
1.7.7.6

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

[libvirt] [PATCH 3/6] unpriv_sgio: Prepare helpers for unpriv_sgio setting

[Osier Yang]

virGetDevice{Major,Minor} could be used across the sources,
but it doesn't relate with this series, and could be done later.

* src/util/util.h: (Declare virGetDevice{Major,Minor}, and
vir{Get,Set}DeviceUnprivSGIO)
* src/util/util.c: (Implement virGetDevice{Major,Minor} and
vir{Get,Set}DeviceUnprivSGIO)
* src/libvirt_private.syms: Export private symbols of upper helpers
---
 src/libvirt_private.syms |4 ++--
 src/util/util.c  |   13 +++--
 src/util/util.h  |8 
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7abaf27..c756130 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1259,7 +1259,7 @@ virFindFileInPath;
 virFormatIntDecimal;
 virGetDeviceMajor;
 virGetDeviceMinor;
-virGetDeviceSGIO;
+virGetDeviceUnprivSGIO;
 virGetGroupID;
 virGetGroupName;
 virGetHostname;
@@ -1278,7 +1278,7 @@ virPipeReadUntilEOF;
 virScaleInteger;
 virSetBlocking;
 virSetCloseExec;
-virSetDeviceSGIO;
+virSetDeviceUnprivSGIO;
 virSetInherit;
 virSetNonBlock;
 virSetUIDGID;
diff --git a/src/util/util.c b/src/util/util.c
index 746f3e9..49619d0 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -3157,8 +3157,8 @@ virGetDeviceMinor(const char *path)
 #endif
 
 int
-virSetDeviceSGIO(const char *path,
- int sgio)
+virSetDeviceUnprivSGIO(const char *path,
+   int unpriv_sgio)
 {
 char *sysfs_path = NULL;
 char *val = NULL;
@@ -3193,7 +3193,7 @@ virSetDeviceSGIO(const char *path,
 goto cleanup;
 }
 
-if (virAsprintf(val, %d, sgio)  0) {
+if (virAsprintf(val, %d, unpriv_sgio)  0) {
 virReportOOMError();
 goto cleanup;
 }
@@ -3211,11 +3211,12 @@ cleanup:
 }
 
 int
-virGetDeviceSGIO(const char *path,
- int *sgio)
+virGetDeviceUnprivSGIO(const char *path,
+   int *unpriv_sgio)
 {
 char *sysfs_path = NULL;
 char *buf = NULL;
+char *tmp = NULL;
 int major;
 int minor;
 int ret = -1;
@@ -3254,7 +3255,7 @@ virGetDeviceSGIO(const char *path,
 *tmp = '\0';
 
 if ((rc = virStrToLong_i(buf, NULL, 10,
- sgio))  0) {
+ unpriv_sgio))  0) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(failed to parse value of %s), sysfs_path);
 goto cleanup;
diff --git a/src/util/util.h b/src/util/util.h
index 0bd9f79..41146c1 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -282,9 +282,9 @@ bool virValidateWWN(const char *wwn);
 
 int virGetDeviceMajor(const char *path);
 int virGetDeviceMinor(const char *path);
-int virSetDeviceSGIO(const char *path,
- int sgio);
-int virGetDeviceSGIO(const char *path,
- int *sgio);
+int virSetDeviceUnprivSGIO(const char *path,
+   int unpriv_sgio);
+int virGetDeviceUnprivSGIO(const char *path,
+   int *unpriv_sgio);
 
 #endif /* __VIR_UTIL_H__ */
-- 
1.7.7.6

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

[libvirt] [PATCH 5/6] unpriv_sgio: Do not restore unpriv_sgio if the disk is being used

[Osier Yang]

This prevents restoring the unpriv_sgio if the disk is shared,
and is being used by other active domain. Because we don't want
to fall into the corruption situation.

* src/conf/domain_conf.h (Declare virDomainDiskIsUsed, which is to
  detect if a disk is using by domain)
* src/conf/domain_conf.c (Implement virDomainDiskIsUsed)
* src/libvirt_private.syms: (Export virDomainDIskIsUsed)
* src/qemu/qemu_process.c (Don't restore unpriv_sgio if the disk
   is shared, and is being used by other
   active domain).
---
 src/conf/domain_conf.c   |   66 ++
 src/conf/domain_conf.h   |4 +++
 src/libvirt_private.syms |1 +
 src/qemu/qemu_process.c  |   11 +++
 4 files changed, 82 insertions(+), 0 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b25229a..02df96e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15697,3 +15697,69 @@ virDomainDefAddSecurityLabelDef(virDomainDefPtr def, 
const char *model)
 
 return seclabel;
 }
+
+struct virDomainDiskIsUsedData {
+char *diskSrc;
+char *name; /* Want to exclude some domain? */
+bool active; /* Want to only iterate the active domains? */
+};
+
+static int
+virDomainObjListSearchDiskPath(const void *payload,
+   const void *name ATTRIBUTE_UNUSED,
+   const void *opaque)
+{
+virDomainObjPtr obj = (virDomainObjPtr)payload;
+const struct virDomainDiskIsUsedData *data = opaque;
+int i;
+int ret = 0;
+
+virDomainObjLock(obj);
+
+if (data-active 
+!virDomainObjIsActive(obj))
+goto cleanup;
+
+if (data-name 
+STREQ(data-name, obj-def-name))
+goto cleanup;
+
+for (i = 0; i  obj-def-ndisks; i++) {
+virDomainDiskDefPtr disk = obj-def-disks[i];
+
+if (STREQ(disk-src, data-diskSrc)) {
+ret = 1;
+goto cleanup;
+}
+}
+
+cleanup:
+virDomainObjUnlock(obj);
+return ret;
+}
+
+/**
+ * virDomainDiskIsUsed:
+ * @doms: List of domain objects
+ * @name: The domain name want to exclude
+ * @diskSrc: The disk path
+ * @active: Whether to exclude inactive domains
+ *
+ * Returns true if the disk is being used. Otherwise returns false.
+ */
+bool
+virDomainDiskIsUsed(const virDomainObjList doms,
+char *name,
+char *diskSrc,
+bool active)
+{
+virDomainObjPtr obj;
+
+struct virDomainDiskIsUsedData data = { diskSrc, name, active };
+
+obj = virHashSearch(doms.objs, virDomainObjListSearchDiskPath, data);
+if (obj)
+return true;
+
+return false;
+}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 1a8de71..3105e05 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1874,6 +1874,10 @@ virDomainObjPtr virDomainFindByUUID(const 
virDomainObjListPtr doms,
 const unsigned char *uuid);
 virDomainObjPtr virDomainFindByName(const virDomainObjListPtr doms,
 const char *name);
+bool virDomainDiskIsUsed(const virDomainObjList doms,
+ char *name,
+ char *diskSrc,
+ bool active);
 
 bool virDomainObjTaint(virDomainObjPtr obj,
enum virDomainTaintFlags taint);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c756130..b9019b7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -319,6 +319,7 @@ virDomainDefFormatInternal;
 virDomainDefFree;
 virDomainDefGetSecurityLabelDef;
 virDomainDiskDefGetSecurityLabelDef;
+virDomainDiskIsUsed;
 virDomainDefAddSecurityLabelDef;
 virDomainDefParseFile;
 virDomainDefParseNode;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e48eed0..eeaaea0 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4120,6 +4120,17 @@ void qemuProcessStop(struct qemud_driver *driver,
 if (!disk-unpriv_sgio)
 continue;
 
+/* Don't try to restore the unpriv_sgio if the disk is shared
+ * by other active domain(s). We don't want to fall into the
+ * corruptions.
+ */
+if (disk-shared 
+virDomainDiskIsUsed(driver-domains,
+vm-def-name,
+disk-src,
+true))
+continue;
+
 if (virSetDeviceUnprivSGIO(disk-src, disk-old_unpriv_sgio)  0)
 VIR_WARN(Unable to restore unpriv_sgio for disk '%s', disk-src);
 }
-- 
1.7.7.6

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

Re: [libvirt] Is DBus a hard dependency

[Daniel P. Berrange]

On Sat, Nov 24, 2012 at 06:34:35PM +0100, Guido Günther wrote:
 Hi,
 currently running libvirtd without DBus fails due to:
 
 error : nwfilterDriverStartup:208 : DBus matches could not be installed. 
 Disabling nwfilter driver
 error : virDBusGetSystemBus:77 : internal error Unable to get DBus system bus 
 connection: Failed to connect to socket /var/run/dbus/system_bus_socket: No 
 such file or directory
 error : virStateInitialize:810 : Initialization of NWFilter state driver 
 failed
 error : daemonRunStateInit:784 : Driver state initialization failed
 
 because we fail driver initialization hard in nwfilter_driver.c:
 
 if (nwfilterDriverInstallDBusMatches(sysbus)  0) {
 VIR_ERROR(_(DBus matches could not be installed. Disabling nwfilter 
   driver));
 /*
  * unfortunately this is fatal since virNWFilterTechDriversInit
  * may have caused the ebiptables driver to use the firewall tool
  * but now that the watches don't work, we just disable the nwfilter
  * driver
  */
 goto error;
 }
 
 I wonder if this on prupose or if we can just make this a soft error and
 go on without DBus? At least in the !HAVE_FIREWALLD case it should be
 o.k. to continue. Shouldn't it? See attached patch.

Generally, if DBus has been requested at compile time, it ought to
be treated as compulsory, otherwise it should be non-fatal.


 From 22571860568bfe8026e60dcede8f332ec6fd002f Mon Sep 17 00:00:00 2001
 Message-Id: 
 22571860568bfe8026e60dcede8f332ec6fd002f.1353774807.git@sigxcpu.org
 From: =?UTF-8?q?Guido=20G=C3=BCnther?= a...@sigxcpu.org
 Date: Sat, 24 Nov 2012 17:32:59 +0100
 Subject: [PATCH] nwfilter: Allow DBus initialization to fail
 To: libvir-list@redhat.com
 
 in case we don't use firewalld. This allows us to run without
 DBus on servers.
 ---
  src/nwfilter/nwfilter_driver.c |2 ++
  1 file changed, 2 insertions(+)
 
 diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
 index 12f47ef..e4f6ec9 100644
 --- a/src/nwfilter/nwfilter_driver.c
 +++ b/src/nwfilter/nwfilter_driver.c
 @@ -204,6 +204,7 @@ nwfilterDriverStartup(int privileged)
   * initializing
   */
  if (nwfilterDriverInstallDBusMatches(sysbus)  0) {
 +#if HAVE_FIREWALLD
  VIR_ERROR(_(DBus matches could not be installed. Disabling nwfilter 
 
driver));
  /*
 @@ -213,6 +214,7 @@ nwfilterDriverStartup(int privileged)
   * driver
   */
  goto error;
 +#endif
  }

IMHO any conditional should be in the nwfilterDriverInstallDBusMatches method
itself. ie that method should be a no-op hardcoded to return 0, if firewalld
has been disabled at compile time.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH] storage: fix logical volume cloning

[Daniel P. Berrange]

On Sun, Nov 25, 2012 at 02:59:33AM +0100, Ján Tomko wrote:
 Commit 258e06c removed setting of the volume type to
 VIR_STORAGE_VOL_BLOCK, which leads to failures in
 storageVolumeCreateXMLFrom.
 
 The type (and target.format) of the volume was set to zero. In
 virStorageBackendGetBuildVolFromFunction, this gets interpreted as
 VIR_STORAGE_FILE_NONE and the qemu-img tool is called with unknown
 none format.
 
 Bug: https://bugzilla.redhat.com/show_bug.cgi?id=879780
 ---
  src/storage/storage_backend_logical.c |2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)
 
 diff --git a/src/storage/storage_backend_logical.c 
 b/src/storage/storage_backend_logical.c
 index de43c3a..fd5cbd1 100644
 --- a/src/storage/storage_backend_logical.c
 +++ b/src/storage/storage_backend_logical.c
 @@ -708,6 +708,8 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
  return -1;
  }
  
 +vol-type = VIR_STORAGE_VOL_BLOCK;
 +
  if (vol-target.path != NULL) {
  /* A target path passed to CreateVol has no meaning */
  VIR_FREE(vol-target.path);

ACK

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH 0/6] Unprivileged SG_IO support

[Osier Yang]

Sorry, please ignore this set, as the patches are mis-orgnized. Will
post a v2 soon.

On 2012年11月26日 18:21, Osier Yang wrote:

Hi,

As a result of RFC [1], this implements the unprivleged SG_IO
support. Though this is a bit rushed, and I still have no chance
to test it yet, but I'd like see an earlier reviewing.

@Paolo, please let me known once you have a kernel build with
the new sysfs knob support. Thanks!

Osier Yang (6):
   unpriv_sgio: Add docs and rng schema for new XML unpriv_sgio
   unpriv_sgio: Parse and format the new XML
   unpriv_sgio: Prepare helpers for unpriv_sgio setting
   unpriv_sgio: Manage unpriv_sgio in domain's lifecyle
   unpriv_sgio: Do not restore unpriv_sgio if the disk is being used
   unpriv_sgio: Error out if the unpriv_sgio setting conflicts with
 others

  docs/formatdomain.html.in  |   10 +-
  docs/schemas/domaincommon.rng  |   52 --
  src/conf/domain_conf.c |  168 ++--
  src/conf/domain_conf.h |   18 ++
  src/libvirt_private.syms   |6 +
  src/qemu/qemu_process.c|   54 +++
  src/util/util.c|  154 ++
  src/util/util.h|7 +
  ...l2argv-disk-scsi-lun-passthrough-upriv-sgio.xml |   32 
  tests/qemuxml2xmltest.c|1 +
  10 files changed, 472 insertions(+), 30 deletions(-)
  create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough-upriv-sgio.xml

[1] https://www.redhat.com/archives/libvir-list/2012-November/msg00988.html

Regards,
Osier

--
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 1/6] unpriv_sgio: Add docs and rng schema for new XML unpriv_sgio

[Daniel P. Berrange]

On Mon, Nov 26, 2012 at 06:21:37PM +0800, Osier Yang wrote:
 Since rawio and unpriv_sgio are only valid for lun, this

I think that's a ugly attribute name, just use 'sgio'.

 groups them together. And since both of them intend to allow
 the unprivledged user to use the SCSI commands, they are must be
 exclusive. Actually unpriv_sgio supersedes rawio, as it
 confines the capability per-device, unlike rawio, which gives
 the domain process broad capablity.

I'd tend to say that the rawio behaviour you describe is
really just a QEMU implementation detail. The XML itself
is written on the basis that the 'rawio' flag is specific
to one device. eg it would be possible to make the 'rawio'
flag work on just a single device if we really wanted to
do the kernel work.

 ---
  docs/formatdomain.html.in |   10 +++-
  docs/schemas/domaincommon.rng |   52 ++
  src/conf/domain_conf.c|   56 
  src/conf/domain_conf.h|   11 +++
  src/libvirt_private.syms  |4 +
  src/qemu/qemu_process.c   |   30 
  src/util/util.c   |  153 
 +
  src/util/util.h   |7 ++
  8 files changed, 293 insertions(+), 30 deletions(-)
 
 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
 index 6a3b976..f3f6a9e 100644
 --- a/docs/formatdomain.html.in
 +++ b/docs/formatdomain.html.in
 @@ -1395,7 +1395,15 @@
  rawio='yes', rawio capability will be enabled for all disks in
  the domain (because, in the case of QEMU, this capability can
  only be set on a per-process basis). This attribute is only
 -valid when device is lun.
 +valid when device is lun. NB, coderawio/code gives
 +the domain process broad capability, to confined the capability
 +as much as possible, one should use codeunpriv_sgio/code
 +instead, which controls the capability per-device.
 +The optional codeunpriv_sgio/code attribute
 +(span class=sincesince 1.0.1/span) indicates whether the
 +disk will allow unprivileged SG_IO, valid settings are yes
 +or no (defaults to no). Note that it's exclusive with
 +attribute coderawio/code;

As above, I think this needs re-wording to clearly differentiate
between the intended meaning of the 'rawio' attribute in general,
vs the current QEMU specific impl of that attribute.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

Re: [libvirt] [PATCH 6/6] unpriv_sgio: Error out if the unpriv_sgio setting conflicts with others

[Daniel P. Berrange]

On Mon, Nov 26, 2012 at 06:21:42PM +0800, Osier Yang wrote:
 This prevent the domain starting if the shared disk's setting conflicts
 with other active domain(s), E.g. A domain with unpriv_sgio set as
 yes, however, another active domain is using it set as no.
 ---
  src/conf/domain_conf.c   |   45 +
  src/conf/domain_conf.h   |3 +++
  src/libvirt_private.syms |1 +
  src/qemu/qemu_process.c  |   14 +-
  4 files changed, 62 insertions(+), 1 deletions(-)
 
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 02df96e..fb7bb1f 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -15763,3 +15763,48 @@ virDomainDiskIsUsed(const virDomainObjList doms,
  
  return false;
  }
 +
 +static int
 +virDomainSharedDiskCompare(const void *payload,
 +   const void *name ATTRIBUTE_UNUSED,
 +   const void *opaque)
 +{
 +virDomainObjPtr obj = (virDomainObjPtr)payload;
 +const virDomainDiskDefPtr disk = (virDomainDiskDefPtr)opaque;
 +int i;
 +
 +virDomainObjLock(obj);
 +
 +/* Ingores the inactive ones */
 +if (!virDomainObjIsActive(obj))
 +return 0;
 +
 +for (i = 0; i  obj-def-ndisks; i++) {
 +if (STRNEQ(obj-def-disks[i]-src, disk-src))
 +continue;
 +
 +if (obj-def-disks[i]-unpriv_sgio != disk-unpriv_sgio) {
 +virDomainObjUnlock(obj);
 +return 1;
 +}
 +}
 +
 +virDomainObjUnlock(obj);
 +return 0;
 +}
 +
 +/* Validate if the shared disk conf is conflicted with other domains,
 + * currently only validates the unpriv_sgio setting
 + */
 +bool
 +virDomainSharedDiskValidate(const virDomainObjList doms,
 +const virDomainDiskDefPtr disk)
 +{
 +virDomainObjPtr obj;
 +
 +obj = virHashSearch(doms.objs, virDomainSharedDiskCompare, disk);
 +if (obj)
 +return false;
 +
 +return true;
 +}

This code is rather undesirable since it requires locking every
single VM known to the QEMU driver. Better to use the model we
have for PCI/USB devices where the driver maintains a central
list of the devices 

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

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

[libvirt] [PATCH 0/6 v2] Unprivileged SG_IO support

[Osier Yang]

Hi,

As a result of RFC [1], this implements the unprivleged SG_IO
support. Though it is a bit rushed, and I still have no chance
to test it yet, but I'd like see an earlier reviewing.

@Paolo, please let me known once you have a kernel build with
the new sysfs knob supported. Thanks!

Osier Yang (6):
  unpriv_sgio: Add docs and rng schema for new XML unpriv_sgio
  unpriv_sgio: Parse and format the new XML
  unpriv_sgio: Prepare helpers for unpriv_sgio setting
  unpriv_sgio: Manage unpriv_sgio in domain's lifecyle
  unpriv_sgio: Do not restore unpriv_sgio if the disk is being used
  unpriv_sgio: Error out if the unpriv_sgio setting conflicts with others

 docs/formatdomain.html.in  |   10 +-
 docs/schemas/domaincommon.rng  |   52 --
 src/conf/domain_conf.c |  168 ++--
 src/conf/domain_conf.h |   18 ++
 src/libvirt_private.syms   |6 +
 src/qemu/qemu_process.c|   54 +++
 src/util/util.c|  154 ++
 src/util/util.h|7 +
 ...l2argv-disk-scsi-lun-passthrough-upriv-sgio.xml |   32 
 tests/qemuxml2xmltest.c|1 +
 10 files changed, 472 insertions(+), 30 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough-upriv-sgio.xml

[1] https://www.redhat.com/archives/libvir-list/2012-November/msg00988.html

Regards,
Osier

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

[libvirt] [PATCH 1/6] unpriv_sgio: Add docs and rng schema for new XML unpriv_sgio

[Osier Yang]

Since rawio and unpriv_sgio are only valid for lun, this
groups them together. And since both of them intend to allow
the unprivledged user to use the SCSI commands, they are must be
exclusive. Actually unpriv_sgio supersedes rawio, as it
confines the capability per-device, unlike rawio, which gives
the domain process broad capablity.
---
 docs/formatdomain.html.in |   10 +++-
 docs/schemas/domaincommon.rng |   52 
 2 files changed, 45 insertions(+), 17 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 6a3b976..f3f6a9e 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1395,7 +1395,15 @@
 rawio='yes', rawio capability will be enabled for all disks in
 the domain (because, in the case of QEMU, this capability can
 only be set on a per-process basis). This attribute is only
-valid when device is lun.
+valid when device is lun. NB, coderawio/code gives
+the domain process broad capability, to confined the capability
+as much as possible, one should use codeunpriv_sgio/code
+instead, which controls the capability per-device.
+The optional codeunpriv_sgio/code attribute
+(span class=sincesince 1.0.1/span) indicates whether the
+disk will allow unprivileged SG_IO, valid settings are yes
+or no (defaults to no). Note that it's exclusive with
+attribute coderawio/code;
 The optional codesnapshot/code attribute indicates the default
 behavior of the disk during disk snapshots: internal
 requires a file format such as qcow2 that can store both the
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 02ad477..7da571c 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -957,24 +957,44 @@
 --
   define name=disk
 element name=disk
-  optional
-attribute name=device
-  choice
-valuefloppy/value
-valuedisk/value
-valuecdrom/value
-valuelun/value
-  /choice
-/attribute
-  /optional
-  optional
-attribute name=rawio
+  choice
+group
+  optional
+attribute name=device
+  choice
+valuefloppy/value
+valuedisk/value
+valuecdrom/value
+  /choice
+/attribute
+  /optional
+/group
+group
+  optional
+attribute name=device
+  valuelun/value
+/attribute
+  /optional
   choice
-valueyes/value
-valueno/value
+optional
+  attribute name=rawio
+choice
+  valueyes/value
+  valueno/value
+/choice
+  /attribute
+/optional
+optional
+  attribute name=unpriv_sgio
+choice
+  valueyes/value
+  valueno/value
+/choice
+  /attribute
+/optional
   /choice
-/attribute
-  /optional
+/group
+  /choice
   optional
 ref name=snapshot/
   /optional
-- 
1.7.7.6

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

[libvirt] [PATCH 2/6] unpriv_sgio: Parse and format the new XML

[Osier Yang]

Setting unpriv_sgio to yes to enable the unprivleged SG_IO,
and no to disable it. Later patch will do the actual setting.
---
 src/conf/domain_conf.c |   57 +++-
 src/conf/domain_conf.h |   10 
 ...l2argv-disk-scsi-lun-passthrough-upriv-sgio.xml |   32 +++
 tests/qemuxml2xmltest.c|1 +
 4 files changed, 87 insertions(+), 13 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-lun-passthrough-upriv-sgio.xml

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ed8b53f..b25229a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -236,6 +236,10 @@ VIR_ENUM_IMPL(virDomainDiskIo, VIR_DOMAIN_DISK_IO_LAST,
   default,
   native,
   threads)
+VIR_ENUM_IMPL(virDomainDiskUnprivSGIO, VIR_DOMAIN_DISK_UNPRIV_SGIO_LAST,
+  default,
+  yes,
+  no)
 VIR_ENUM_IMPL(virDomainIoEventFd, VIR_DOMAIN_IO_EVENT_FD_LAST,
   default,
   on,
@@ -3515,6 +3519,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
 char *device = NULL;
 char *snapshot = NULL;
 char *rawio = NULL;
+char *unpriv_sgio = NULL;
 char *driverName = NULL;
 char *driverType = NULL;
 char *source = NULL;
@@ -3576,6 +3581,7 @@ virDomainDiskDefParseXML(virCapsPtr caps,
 snapshot = virXMLPropString(node, snapshot);
 
 rawio = virXMLPropString(node, rawio);
+unpriv_sgio = virXMLPropString(node, unpriv_sgio);
 
 cur = node-children;
 while (cur != NULL) {
@@ -3966,24 +3972,45 @@ virDomainDiskDefParseXML(virCapsPtr caps,
 def-snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE;
 }
 
+if (rawio  unpriv_sgio) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(rawio and unpriv_sgio are exclusive));
+goto error;
+}
+
+if ((rawio || unpriv_sgio) 
+(def-device != VIR_DOMAIN_DISK_DEVICE_LUN)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(rawio can be used only with device='lun'));
+goto error;
+}
+
 if (rawio) {
 def-rawio_specified = true;
-if (def-device == VIR_DOMAIN_DISK_DEVICE_LUN) {
-if (STREQ(rawio, yes)) {
-def-rawio = 1;
-} else if (STREQ(rawio, no)) {
-def-rawio = 0;
-} else {
-virReportError(VIR_ERR_XML_ERROR,
-   _(unknown disk rawio setting '%s'),
-   rawio);
-goto error;
-}
+if (STREQ(rawio, yes)) {
+def-rawio = 1;
+} else if (STREQ(rawio, no)) {
+def-rawio = 0;
 } else {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
-   _(rawio can be used only with device='lun'));
+virReportError(VIR_ERR_XML_ERROR,
+   _(unknown disk rawio setting '%s'),
+   rawio);
+goto error;
+}
+}
+
+if (unpriv_sgio) {
+int unpriv_sgio_val = 0;
+
+if ((unpriv_sgio_val =
+ virDomainDiskUnprivSGIOTypeFromString(unpriv_sgio))  0) {
+virReportError(VIR_ERR_XML_ERROR,
+   _(unknown disk unpriv_sgio setting '%s'),
+   unpriv_sgio);
 goto error;
 }
+
+def-unpriv_sgio = unpriv_sgio_val;
 }
 
 if (bus) {
@@ -4220,6 +4247,7 @@ cleanup:
 VIR_FREE(type);
 VIR_FREE(snapshot);
 VIR_FREE(rawio);
+VIR_FREE(unpriv_sgio);
 VIR_FREE(target);
 VIR_FREE(source);
 VIR_FREE(tray);
@@ -11897,6 +11925,9 @@ virDomainDiskDefFormat(virBufferPtr buf,
 virBufferAddLit(buf,  rawio='no');
 }
 }
+if (def-unpriv_sgio)
+virBufferAsprintf(buf,  unpriv_sgio='%s',
+  
virDomainDiskUnprivSGIOTypeToString(def-unpriv_sgio));
 if (def-snapshot 
 !(def-snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE  def-readonly))
 virBufferAsprintf(buf,  snapshot='%s',
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index c3e8c16..105fb7d 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -496,6 +496,14 @@ enum  virDomainDiskIo {
 VIR_DOMAIN_DISK_IO_LAST
 };
 
+enum virDomainDiskUnprivSGIO {
+VIR_DOMAIN_DISK_UNPRIV_SGIO_DEFAULT = 0,
+VIR_DOMAIN_DISK_UNPRIV_SGIO_YES,
+VIR_DOMAIN_DISK_UNPRIV_SGIO_NO,
+
+VIR_DOMAIN_DISK_UNPRIV_SGIO_LAST
+};
+
 enum virDomainIoEventFd {
 VIR_DOMAIN_IO_EVENT_FD_DEFAULT = 0,
 VIR_DOMAIN_IO_EVENT_FD_ON,
@@ -607,6 +615,7 @@ struct _virDomainDiskDef {
 virStorageEncryptionPtr encryption;
 bool rawio_specified;
 int rawio; /* no = 0, yes = 1 */
+int unpriv_sgio;  /* no = 0, yes = 1 */
 
 size_t nseclabels;
 virSecurityDeviceLabelDefPtr *seclabels;
@@ 

[libvirt] [PATCH 5/6] unpriv_sgio: Do not restore unpriv_sgio if the disk is being used

[Osier Yang]

This prevents restoring the unpriv_sgio if the disk is shared,
and is being used by other active domain. Because we don't want
to fall into the corruption situation.

* src/conf/domain_conf.h (Declare virDomainDiskIsUsed, which is to
  detect if a disk is using by domain)
* src/conf/domain_conf.c (Implement virDomainDiskIsUsed)
* src/libvirt_private.syms: (Export virDomainDIskIsUsed)
* src/qemu/qemu_process.c (Don't restore unpriv_sgio if the disk
   is shared, and is being used by other
   active domain).
---
 src/conf/domain_conf.c   |   66 ++
 src/conf/domain_conf.h   |4 +++
 src/libvirt_private.syms |1 +
 src/qemu/qemu_process.c  |   11 +++
 4 files changed, 82 insertions(+), 0 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b25229a..02df96e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15697,3 +15697,69 @@ virDomainDefAddSecurityLabelDef(virDomainDefPtr def, 
const char *model)
 
 return seclabel;
 }
+
+struct virDomainDiskIsUsedData {
+char *diskSrc;
+char *name; /* Want to exclude some domain? */
+bool active; /* Want to only iterate the active domains? */
+};
+
+static int
+virDomainObjListSearchDiskPath(const void *payload,
+   const void *name ATTRIBUTE_UNUSED,
+   const void *opaque)
+{
+virDomainObjPtr obj = (virDomainObjPtr)payload;
+const struct virDomainDiskIsUsedData *data = opaque;
+int i;
+int ret = 0;
+
+virDomainObjLock(obj);
+
+if (data-active 
+!virDomainObjIsActive(obj))
+goto cleanup;
+
+if (data-name 
+STREQ(data-name, obj-def-name))
+goto cleanup;
+
+for (i = 0; i  obj-def-ndisks; i++) {
+virDomainDiskDefPtr disk = obj-def-disks[i];
+
+if (STREQ(disk-src, data-diskSrc)) {
+ret = 1;
+goto cleanup;
+}
+}
+
+cleanup:
+virDomainObjUnlock(obj);
+return ret;
+}
+
+/**
+ * virDomainDiskIsUsed:
+ * @doms: List of domain objects
+ * @name: The domain name want to exclude
+ * @diskSrc: The disk path
+ * @active: Whether to exclude inactive domains
+ *
+ * Returns true if the disk is being used. Otherwise returns false.
+ */
+bool
+virDomainDiskIsUsed(const virDomainObjList doms,
+char *name,
+char *diskSrc,
+bool active)
+{
+virDomainObjPtr obj;
+
+struct virDomainDiskIsUsedData data = { diskSrc, name, active };
+
+obj = virHashSearch(doms.objs, virDomainObjListSearchDiskPath, data);
+if (obj)
+return true;
+
+return false;
+}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 1a8de71..3105e05 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1874,6 +1874,10 @@ virDomainObjPtr virDomainFindByUUID(const 
virDomainObjListPtr doms,
 const unsigned char *uuid);
 virDomainObjPtr virDomainFindByName(const virDomainObjListPtr doms,
 const char *name);
+bool virDomainDiskIsUsed(const virDomainObjList doms,
+ char *name,
+ char *diskSrc,
+ bool active);
 
 bool virDomainObjTaint(virDomainObjPtr obj,
enum virDomainTaintFlags taint);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c756130..b9019b7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -319,6 +319,7 @@ virDomainDefFormatInternal;
 virDomainDefFree;
 virDomainDefGetSecurityLabelDef;
 virDomainDiskDefGetSecurityLabelDef;
+virDomainDiskIsUsed;
 virDomainDefAddSecurityLabelDef;
 virDomainDefParseFile;
 virDomainDefParseNode;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e48eed0..eeaaea0 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4120,6 +4120,17 @@ void qemuProcessStop(struct qemud_driver *driver,
 if (!disk-unpriv_sgio)
 continue;
 
+/* Don't try to restore the unpriv_sgio if the disk is shared
+ * by other active domain(s). We don't want to fall into the
+ * corruptions.
+ */
+if (disk-shared 
+virDomainDiskIsUsed(driver-domains,
+vm-def-name,
+disk-src,
+true))
+continue;
+
 if (virSetDeviceUnprivSGIO(disk-src, disk-old_unpriv_sgio)  0)
 VIR_WARN(Unable to restore unpriv_sgio for disk '%s', disk-src);
 }
-- 
1.7.7.6

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

[libvirt] [PATCH 4/6] unpriv_sgio: Manage unpriv_sgio in domain's lifecyle

[Osier Yang]

To record the original unpriv_sgio value, this introduces
old_unpriv_sgio for disk def. When the domain is starting,
the disk's unpriv_sgio is set with regards to the config
in domain XML. And when the domain is being destroyed, it's
restored to the original value (old_unpriv_sgio)
---
 src/conf/domain_conf.h  |1 +
 src/qemu/qemu_process.c |   31 +++
 2 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 105fb7d..1a8de71 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -616,6 +616,7 @@ struct _virDomainDiskDef {
 bool rawio_specified;
 int rawio; /* no = 0, yes = 1 */
 int unpriv_sgio;  /* no = 0, yes = 1 */
+int old_unpriv_sgio; /* To record the old unpriv_sgio value, internally */
 
 size_t nseclabels;
 virSecurityDeviceLabelDefPtr *seclabels;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 3d7a5a0..e48eed0 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3703,6 +3703,26 @@ int qemuProcessStart(virConnectPtr conn,
 virCommandAllowCap(cmd, CAP_SYS_RAWIO);
 }
 
+/* Set unpriv_sgio for disks */
+for (i = 0; i  vm-def-ndisks; i++) {
+virDomainDiskDefPtr disk = vm-def-disks[i];
+int old_unpriv_sgio;
+
+if (!disk-unpriv_sgio)
+continue;
+
+if (virGetDeviceUnprivSGIO(disk-src, old_unpriv_sgio)  0)
+goto cleanup;
+
+disk-old_unpriv_sgio = old_unpriv_sgio;
+
+if (virSetDeviceUnprivSGIO(disk-src,
+   (disk-unpriv_sgio ==
+VIR_DOMAIN_DISK_UNPRIV_SGIO_YES)
+   ? 1 : 0)  0)
+goto cleanup;
+}
+
 virCommandSetPreExecHook(cmd, qemuProcessHook, hookData);
 
 virCommandSetOutputFD(cmd, logfile);
@@ -4093,6 +4113,17 @@ void qemuProcessStop(struct qemud_driver *driver,
   flags  
VIR_QEMU_PROCESS_STOP_MIGRATED);
 virSecurityManagerReleaseLabel(driver-securityManager, vm-def);
 
+/* Restore disk's unpriv_sgio */
+for (i = 0; i  vm-def-ndisks; i++) {
+virDomainDiskDefPtr disk = vm-def-disks[i];
+
+if (!disk-unpriv_sgio)
+continue;
+
+if (virSetDeviceUnprivSGIO(disk-src, disk-old_unpriv_sgio)  0)
+VIR_WARN(Unable to restore unpriv_sgio for disk '%s', disk-src);
+}
+
 /* Clear out dynamically assigned labels */
 for (i = 0; i  vm-def-nseclabels; i++) {
 if (vm-def-seclabels[i]-type == VIR_DOMAIN_SECLABEL_DYNAMIC) {
-- 
1.7.7.6

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

[libvirt] [PATCH 3/6] unpriv_sgio: Prepare helpers for unpriv_sgio setting

[Osier Yang]

virGetDevice{Major,Minor} could be used across the sources,
but it doesn't relate with this series, and could be done later.

* src/util/util.h: (Declare virGetDevice{Major,Minor}, and
vir{Get,Set}DeviceUnprivSGIO)
* src/util/util.c: (Implement virGetDevice{Major,Minor} and
vir{Get,Set}DeviceUnprivSGIO)
* src/libvirt_private.syms: Export private symbols of upper helpers
---
 src/libvirt_private.syms |4 +
 src/util/util.c  |  154 ++
 src/util/util.h  |7 ++
 3 files changed, 165 insertions(+), 0 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0115db1..c756130 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1257,6 +1257,9 @@ virFileWaitForDevices;
 virFileWriteStr;
 virFindFileInPath;
 virFormatIntDecimal;
+virGetDeviceMajor;
+virGetDeviceMinor;
+virGetDeviceUnprivSGIO;
 virGetGroupID;
 virGetGroupName;
 virGetHostname;
@@ -1275,6 +1278,7 @@ virPipeReadUntilEOF;
 virScaleInteger;
 virSetBlocking;
 virSetCloseExec;
+virSetDeviceUnprivSGIO;
 virSetInherit;
 virSetNonBlock;
 virSetUIDGID;
diff --git a/src/util/util.c b/src/util/util.c
index 2fd0f2c..49619d0 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -3113,3 +3113,157 @@ virValidateWWN(const char *wwn) {
 
 return true;
 }
+
+#if defined(major)  defined(minor)
+int
+virGetDeviceMajor(const char *path)
+{
+struct stat sb;
+
+if (stat(path, sb)  0)
+return -errno;
+
+if (!S_ISBLK(sb.st_mode))
+return -EINVAL;
+
+return major(sb.st_rdev);
+}
+
+int
+virGetDeviceMinor(const char *path)
+{
+struct stat sb;
+
+if (stat(path, sb)  0)
+return -errno;
+
+if (!S_ISBLK(sb.st_mode))
+return -EINVAL;
+
+return minor(sb.st_rdev);
+}
+#else
+int
+virGetDeviceMajor(const char *path)
+{
+return -ENOSYS;
+}
+
+int
+virGetDeviceMinor(const char *path)
+{
+return -ENOSYS;
+}
+#endif
+
+int
+virSetDeviceUnprivSGIO(const char *path,
+   int unpriv_sgio)
+{
+char *sysfs_path = NULL;
+char *val = NULL;
+int major;
+int minor;
+int ret = -1;
+int rc = -1;
+
+if ((major = virGetDeviceMajor(path))  0) {
+virReportSystemError(-major,
+ _(Unable to get major number of device '%s'),
+ path);
+return -1;
+}
+
+if ((minor = virGetDeviceMinor(path))  0) {
+virReportSystemError(-minor,
+ _(Unable to get minor number of device '%s'),
+ path);
+return -1;
+}
+
+if (virAsprintf(sysfs_path, /sys/dev/block/%d:%d/queue/unpriv_sgio,
+major, minor)  0) {
+virReportOOMError();
+return -1;
+}
+
+if (!virFileExists(sysfs_path)) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+   _(unpriv_sgio is not supported by this kernel));
+goto cleanup;
+}
+
+if (virAsprintf(val, %d, unpriv_sgio)  0) {
+virReportOOMError();
+goto cleanup;
+}
+
+if ((rc = virFileWriteStr(sysfs_path, val, 0))  0) {
+virReportSystemError(-rc, _(failed to set %s), sysfs_path);
+goto cleanup;
+}
+
+ret = 0;
+cleanup:
+VIR_FREE(sysfs_path);
+VIR_FREE(val);
+return ret;
+}
+
+int
+virGetDeviceUnprivSGIO(const char *path,
+   int *unpriv_sgio)
+{
+char *sysfs_path = NULL;
+char *buf = NULL;
+char *tmp = NULL;
+int major;
+int minor;
+int ret = -1;
+int rc = -1;
+
+if ((major = virGetDeviceMajor(path))  0) {
+virReportSystemError(-major,
+ _(Unable to get major number of device '%s'),
+ path);
+return -1;
+}
+
+if ((minor = virGetDeviceMinor(path))  0) {
+virReportSystemError(-minor,
+ _(Unable to get minor number of device '%s'),
+ path);
+return -1;
+}
+
+if (virAsprintf(sysfs_path, /sys/dev/block/%d:%d/queue/unpriv_sgio,
+major, minor)  0) {
+virReportOOMError();
+return -1;
+}
+
+if (!virFileExists(sysfs_path)) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+   _(unpriv_sgio is not supported by this kernel));
+goto cleanup;
+}
+
+if (virFileReadAll(sysfs_path, 1024, buf)  0)
+goto cleanup;
+
+if ((tmp = strchr(buf, '\n')))
+*tmp = '\0';
+
+if ((rc = virStrToLong_i(buf, NULL, 10,
+ unpriv_sgio))  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(failed to parse value of %s), sysfs_path);
+goto cleanup;
+}
+
+ret = 0;
+cleanup:
+VIR_FREE(sysfs_path);
+VIR_FREE(buf);
+return ret;
+}
diff --git a/src/util/util.h b/src/util/util.h
index 

[libvirt] [PATCH 6/6] unpriv_sgio: Error out if the unpriv_sgio setting conflicts with others

[Osier Yang]

This prevents the domain starting if the shared disk's setting conflicts
with other active domain(s), E.g. A domain with unpriv_sgio set as
yes, however, another active domain is using it set as no.
---
 src/conf/domain_conf.c   |   45 +
 src/conf/domain_conf.h   |3 +++
 src/libvirt_private.syms |1 +
 src/qemu/qemu_process.c  |   14 +-
 4 files changed, 62 insertions(+), 1 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 02df96e..fb7bb1f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15763,3 +15763,48 @@ virDomainDiskIsUsed(const virDomainObjList doms,
 
 return false;
 }
+
+static int
+virDomainSharedDiskCompare(const void *payload,
+   const void *name ATTRIBUTE_UNUSED,
+   const void *opaque)
+{
+virDomainObjPtr obj = (virDomainObjPtr)payload;
+const virDomainDiskDefPtr disk = (virDomainDiskDefPtr)opaque;
+int i;
+
+virDomainObjLock(obj);
+
+/* Ingores the inactive ones */
+if (!virDomainObjIsActive(obj))
+return 0;
+
+for (i = 0; i  obj-def-ndisks; i++) {
+if (STRNEQ(obj-def-disks[i]-src, disk-src))
+continue;
+
+if (obj-def-disks[i]-unpriv_sgio != disk-unpriv_sgio) {
+virDomainObjUnlock(obj);
+return 1;
+}
+}
+
+virDomainObjUnlock(obj);
+return 0;
+}
+
+/* Validate if the shared disk conf is conflicted with other domains,
+ * currently only validates the unpriv_sgio setting
+ */
+bool
+virDomainSharedDiskValidate(const virDomainObjList doms,
+const virDomainDiskDefPtr disk)
+{
+virDomainObjPtr obj;
+
+obj = virHashSearch(doms.objs, virDomainSharedDiskCompare, disk);
+if (obj)
+return false;
+
+return true;
+}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3105e05..35f46d2 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1878,6 +1878,9 @@ bool virDomainDiskIsUsed(const virDomainObjList doms,
  char *name,
  char *diskSrc,
  bool active);
+bool
+virDomainSharedDiskValidate(const virDomainObjList doms,
+const virDomainDiskDefPtr disk);
 
 bool virDomainObjTaint(virDomainObjPtr obj,
enum virDomainTaintFlags taint);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b9019b7..b14eb40 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -489,6 +489,7 @@ virDomainSaveStatus;
 virDomainSaveXML;
 virDomainSeclabelTypeFromString;
 virDomainSeclabelTypeToString;
+virDomainSharedDiskValidate;
 virDomainShutdownReasonTypeFromString;
 virDomainShutdownReasonTypeToString;
 virDomainShutoffReasonTypeFromString;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index eeaaea0..b5676e0 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3711,6 +3711,18 @@ int qemuProcessStart(virConnectPtr conn,
 if (!disk-unpriv_sgio)
 continue;
 
+ /* Error out if the disk is shared, but the unpriv_sgio conflicts
+  * with other active domain(s).
+  */
+if (disk-shared 
+!virDomainSharedDiskValidate(driver-domains,
+ disk)) {
+virReportError(VIR_ERR_XML_ERROR,
+   _(Setting of shared disk '%s' conflicts with 
+ other active domains), disk-src);
+goto cleanup;
+}
+
 if (virGetDeviceUnprivSGIO(disk-src, old_unpriv_sgio)  0)
 goto cleanup;
 
@@ -3719,7 +3731,7 @@ int qemuProcessStart(virConnectPtr conn,
 if (virSetDeviceUnprivSGIO(disk-src,
(disk-unpriv_sgio ==
 VIR_DOMAIN_DISK_UNPRIV_SGIO_YES)
-   ? 1 : 0)  0)
+? 1 : 0)  0)
 goto cleanup;
 }
 
-- 
1.7.7.6

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

Re: [libvirt] libvirt [PATCHv2 0/2] Refactor ESX storage driver and append support for iSCSI devices.

[Matthias Bolte]

2012/11/25 Ata Bohra ata.hus...@hotmail.com:
 Hi All,

 Checking it if someone can spend sometime reviewing the below patches:

 [libvirt] libvirt [PATCHv2 0/2] Refactor ESX storage driver and append
 support for iSCSI devices. Sat Nov 10 07:18:38 GMT 2012
 [libvirt] libvirt [PATCHv2 2/2] Add iSCSI backend storage driver for ESX.
 Sat Nov 10 07:18:49 GMT 2012
 [libvirt] libvirt [PATCHv2 1/2] Refactor ESX storage driver to implement
 facade pattern Sat Nov 10 07:18:49 GMT 2012

 Thanks for support!

 Regards,
 Ata

I'm taking care of that today... finally :)

Regards,
Matthias

 From: ata.hus...@hotmail.com
 To: libvir-list@redhat.com
 CC: ata.hus...@hotmail.com
 Subject: libvirt [PATCHv2 0/2] Refactor ESX storage driver and append
 support for iSCSI devices.
 Date: Fri, 9 Nov 2012 23:18:06 -0800


 Major changes:
 Split changes to two parts:
 a. Refactor ESX storage driver to implement facade design.
 b. Add backend driver to support iSCSI devices.

 Ata E Husain Bohra (2):
 Refactor ESX storage driver to implement facade pattern
 Add iSCSI backend storage driver for ESX.

 daemon/remote.c | 6 +-
 src/Makefile.am | 2 +
 src/conf/storage_conf.c | 3 +-
 src/datatypes.c | 25 +-
 src/datatypes.h | 24 +-
 src/esx/esx_driver.c | 6 +-
 src/esx/esx_storage_backend_iscsi.c | 807 +++
 src/esx/esx_storage_backend_iscsi.h | 29 +
 src/esx/esx_storage_backend_vmfs.c | 1491
 +++
 src/esx/esx_storage_backend_vmfs.h | 30 +
 src/esx/esx_storage_driver.c | 1325 +--
 src/esx/esx_vi.c | 337 +++-
 src/esx/esx_vi.h | 21 +-
 src/esx/esx_vi_generator.input | 302 +++
 src/esx/esx_vi_generator.py | 19 +
 src/parallels/parallels_storage.c | 24 +-
 src/remote/remote_driver.c | 6 +-
 src/storage/storage_driver.c | 28 +-
 src/test/test_driver.c | 30 +-
 src/vbox/vbox_tmpl.c | 14 +-
 20 files changed, 3357 insertions(+), 1172 deletions(-)
 create mode 100644 src/esx/esx_storage_backend_iscsi.c
 create mode 100644 src/esx/esx_storage_backend_iscsi.h
 create mode 100644 src/esx/esx_storage_backend_vmfs.c
 create mode 100644 src/esx/esx_storage_backend_vmfs.h

 --
 1.7.9.5


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



-- 
Matthias Bolte
http://photron.blogspot.com

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

[libvirt] [PATCH] lxc: Avoid segfault of libvirt_lxc helper on early cleanup paths

[Peter Krempa]

Early jumps to the cleanup label caused a crash of the libvirt_lxc
container helper as the cleanup section called
virLXCControllerDeleteInterfaces(ctrl) without checking the ctrl argument
for NULL. The argument was de-referenced soon after.

$ /usr/libexec/libvirt_lxc
/usr/libexec/libvirt_lxc: missing --name argument for configuration
Segmentation fault
---
 src/lxc/lxc_controller.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index a9d2d40..ea5bc58 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -1672,7 +1672,8 @@ int main(int argc, char *argv[])

 cleanup:
 virPidFileDelete(LXC_STATE_DIR, name);
-virLXCControllerDeleteInterfaces(ctrl);
+if (ctrl)
+virLXCControllerDeleteInterfaces(ctrl);
 for (i = 0 ; i  nttyFDs ; i++)
 VIR_FORCE_CLOSE(ttyFDs[i]);
 VIR_FREE(ttyFDs);
-- 
1.8.0

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

Re: [libvirt] [PATCH 1/6] unpriv_sgio: Add docs and rng schema for new XML unpriv_sgio

[Paolo Bonzini]

Il 26/11/2012 11:38, Daniel P. Berrange ha scritto:
  Since rawio and unpriv_sgio are only valid for lun, this
 I think that's a ugly attribute name, just use 'sgio'.

That doesn't explain what it is for.  Some possibilities:

- filter-scsi-commands=yes/no

- cdb-filter=yes/no

- privileged-scsi-commands=allow/deny

- cdb=allow-all/filter

Paolo

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

Re: [libvirt] [PATCH] build: fix build --without-network

[Michal Privoznik]

On 21.11.2012 15:28, Eric Blake wrote:
 On 11/21/2012 06:59 AM, Ján Tomko wrote:
 bridge_driver.h: silence gcc warnings:
 statement with no effect [-Wunused-value]
 unused variable 'net' [-Wunused-variable]

 virdrivermoduletest.c: don't require network driver module
 if it hasn't been built.
 ---
  src/network/bridge_driver.h |4 ++--
  tests/virdrivermoduletest.c |8 
  2 files changed, 10 insertions(+), 2 deletions(-)

 diff --git a/src/network/bridge_driver.h b/src/network/bridge_driver.h
 index 0fae275..1c42d81 100644
 --- a/src/network/bridge_driver.h
 +++ b/src/network/bridge_driver.h
 @@ -53,8 +53,8 @@ int networkBuildDhcpDaemonCommandLine(virNetworkObjPtr 
 network,
  # else
  /* Define no-op replacements that don't drag in any link dependencies.  */
  #  define networkAllocateActualDevice(iface) 0
 -#  define networkNotifyActualDevice(iface) 0
 -#  define networkReleaseActualDevice(iface) 0
 +#  define networkNotifyActualDevice(iface) (iface=iface,0)
 +#  define networkReleaseActualDevice(iface) (iface=iface,0)
 
 I would write this as (iface = iface, 0) for consistent operator spacing
 with the rest of our code.
 
 +++ b/tests/virdrivermoduletest.c
 @@ -83,10 +83,18 @@ mymain(void)
  TEST(interface, NULL);
  #endif
  #ifdef WITH_QEMU
 +# ifdef WITH_NETWORK
  TEST(qemu, network);
 +# else
 +TEST(qemu, NULL);
 +# endif
  #endif
  #ifdef WITH_LXC
 +# ifdef WITH_NETWORK
  TEST(lxc, network);
 +# else
 +TEST(lxc, NULL);
 +# endif
  #endif
  #ifdef WITH_UML
  TEST(uml, NULL);
 
 Is it any easier to do:
 
 #ifdef WITH_NETWORK
 # define USE_NETWORK network
 #else
 # define USE_NETWORK NULL
 #endif
 
 rather than repeated nested #ifdef?  But that's cosmetic, so I don't
 care if you don't change it.
 
 ACK with the formatting nit fixed.
 
 
 

Fixed and pushed.

Michal

I've squashed this in:

diff --git a/src/network/bridge_driver.h b/src/network/bridge_driver.h
index 1c42d81..638a6ac 100644
--- a/src/network/bridge_driver.h
+++ b/src/network/bridge_driver.h
@@ -53,8 +53,8 @@ int networkBuildDhcpDaemonCommandLine(virNetworkObjPtr
network,
 # else
 /* Define no-op replacements that don't drag in any link dependencies.  */
 #  define networkAllocateActualDevice(iface) 0
-#  define networkNotifyActualDevice(iface) (iface=iface,0)
-#  define networkReleaseActualDevice(iface) (iface=iface,0)
+#  define networkNotifyActualDevice(iface) (iface=iface, 0)
+#  define networkReleaseActualDevice(iface) (iface=iface, 0)
 #  define networkGetNetworkAddress(netname, netaddr) (-2)
 #  define networkBuildDhcpDaemonCommandLine(network, cmdout, pidfile,
dctx) 0
 # endif
diff --git a/tests/virdrivermoduletest.c b/tests/virdrivermoduletest.c
index 0123c36..53a772e 100644
--- a/tests/virdrivermoduletest.c
+++ b/tests/virdrivermoduletest.c
@@ -65,7 +65,10 @@ mymain(void)
 virDriverModuleInitialize(abs_builddir /../src/.libs);

 #ifdef WITH_NETWORK
+# define USE_NETWORK network
 TEST(network, NULL);
+#else
+# define USE_NETWORK NULL
 #endif
 #ifdef WITH_STORAGE
 TEST(storage, NULL);
@@ -83,18 +86,10 @@ mymain(void)
 TEST(interface, NULL);
 #endif
 #ifdef WITH_QEMU
-# ifdef WITH_NETWORK
-TEST(qemu, network);
-# else
-TEST(qemu, NULL);
-# endif
+TEST(qemu, USE_NETWORK);
 #endif
 #ifdef WITH_LXC
-# ifdef WITH_NETWORK
-TEST(lxc, network);
-# else
-TEST(lxc, NULL);
-# endif
+TEST(lxc, USE_NETWORK);
 #endif
 #ifdef WITH_UML
 TEST(uml, NULL);

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

Re: [libvirt] [PATCH] storage: fix logical volume cloning

[Michal Privoznik]

On 26.11.2012 11:34, Daniel P. Berrange wrote:
 On Sun, Nov 25, 2012 at 02:59:33AM +0100, Ján Tomko wrote:
 Commit 258e06c removed setting of the volume type to
 VIR_STORAGE_VOL_BLOCK, which leads to failures in
 storageVolumeCreateXMLFrom.

 The type (and target.format) of the volume was set to zero. In
 virStorageBackendGetBuildVolFromFunction, this gets interpreted as
 VIR_STORAGE_FILE_NONE and the qemu-img tool is called with unknown
 none format.

 Bug: https://bugzilla.redhat.com/show_bug.cgi?id=879780
 ---
  src/storage/storage_backend_logical.c |2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)

 diff --git a/src/storage/storage_backend_logical.c 
 b/src/storage/storage_backend_logical.c
 index de43c3a..fd5cbd1 100644
 --- a/src/storage/storage_backend_logical.c
 +++ b/src/storage/storage_backend_logical.c
 @@ -708,6 +708,8 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
  return -1;
  }
  
 +vol-type = VIR_STORAGE_VOL_BLOCK;
 +
  if (vol-target.path != NULL) {
  /* A target path passed to CreateVol has no meaning */
  VIR_FREE(vol-target.path);
 
 ACK
 
 Daniel
 

Pushed now.

Michal

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

Re: [libvirt] [PATCH] cpu: Add Intel Haswell cpu model

[Peter Krempa]

On 11/23/12 09:16, Martin Kletzander wrote:

On 11/22/2012 04:48 PM, Peter Krempa wrote:

On 11/22/12 15:52, Martin Kletzander wrote:

On 11/22/2012 03:05 PM, Peter Krempa wrote:

[...]

According to the qemu patch, the model should be only adding features,
but I see rdtscp disappeared between SandyBridge and Haswell.  The
question is whether this is QEMU bug or not, do you have any info on
that?  If not, maybe we should cross-post ask in qemu-devel.

We also include 'sep' and 'fpu' on top of these things, but I recall
some conversation about qemu dropping 'sep' from some models lately, but
I have no idea about 'fpu' flag handling there either.


Thanks for pointing that out on the qemu-devel list:

https://lists.gnu.org/archive/html/qemu-devel/2012-11/msg02400.html



I think we can safely assume the patch will make it in a few days, but
to be sure I'm giving you an ACK for when the patch gets into qemu's
upstream.

Martin


I pushed the patch now. Eduardo's patch got an ack and should make it 
into qemu 1.3.


Peter

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

Re: [libvirt] [test-API][PATCH] Add managedsave test cases

[Guannan Ren]

On 11/22/2012 11:56 AM, hongming wrote:

The managedsave test cases and test suite cover test include
verifying virsh commands managedsave(include all flags and their
combination)/managedsave-remove and managedSaveRemove/ManagedSave/
hasManagedSaveImage python APIs.
The following new files be created.
new file: cases/managedsave.conf
- Test all test cases
new file: repos/managedsave/__init__.py
new file: repos/managedsave/managedsave.py
- Test mangaedsave command/API and all flags
new file: repos/managedsave/managedsave_remove.py
- Test managedsave-remove command/API
new file: repos/managedsave/managedsave_start.py
- Verfiy managedsave'flags and start from managedsave image
---
  cases/managedsave.conf  |   63 
  repos/managedsave/managedsave.py|  162 +++
  repos/managedsave/managedsave_remove.py |   61 
  repos/managedsave/managedsave_start.py  |  152 +
  4 files changed, 438 insertions(+), 0 deletions(-)
  create mode 100644 cases/managedsave.conf
  create mode 100644 repos/managedsave/__init__.py
  create mode 100644 repos/managedsave/managedsave.py
  create mode 100644 repos/managedsave/managedsave_remove.py
  create mode 100644 repos/managedsave/managedsave_start.py

diff --git a/cases/managedsave.conf b/cases/managedsave.conf
new file mode 100644
index 000..8dcafe2
--- /dev/null
+++ b/cases/managedsave.conf
@@ -0,0 +1,63 @@
+domain:install_linux_cdrom
+guestname
+$defaultname
+guestos
+$defaultos
+guestarch
+$defaultarch
+vcpu
+$defaultvcpu
+memory
+$defaultmem
+hddriver
+$defaulthd
+nicdriver
+$defaultnic
+imageformat
+qcow2
+macaddr
+54:52:00:4a:16:30
+
+#VIR_DOMAIN_SAVE_BYPASS_CACHE = 1
+#VIR_DOMAIN_SAVE_RUNNING = 2
+#VIR_DOMAIN_SAVE_PAUSED = 4
+#No_FLAGS = 0
+managedsave:managedsave
+guestname
+$defaultname
+flags
+1|2
+
+managedsave:managedsave_start
+guestname
+$defaultname
+flags
+noping
+
+managedsave:managedsave
+guestname
+$defaultname
+flags
+1|4
+
+managedsave:managedsave_start
+guestname
+$defaultname
+flags
+noping
+
+managedsave:managedsave
+guestname
+$defaultname
+flags
+0
+
+managedsave:managedsave_remove
+guestname
+$defaultname
+
+managedsave:managedsave_start
+guestname
+$defaultname
+flags
+noping
diff --git a/repos/managedsave/__init__.py b/repos/managedsave/__init__.py
new file mode 100644
index 000..e69de29
diff --git a/repos/managedsave/managedsave.py b/repos/managedsave/managedsave.py
new file mode 100644
index 000..5e7c105
--- /dev/null
+++ b/repos/managedsave/managedsave.py
@@ -0,0 +1,162 @@
+#!/usr/bin/env python
+
+import os
+import math
+
+import libvirt
+from libvirt import libvirtError
+
+from src import sharedmod
+from utils import utils
+
+required_params = ('guestname', 'flags',)
+optional_params = {}
+
+def check_guest_status(*args):
+Check guest current status
+(domobj, logger) = args
+state = domobj.info()[0]
+logger.debug(current guest status: %s % state)
+
+if state == libvirt.VIR_DOMAIN_SHUTOFF or \
+   state == libvirt.VIR_DOMAIN_SHUTDOWN or \
+   state == libvirt.VIR_DOMAIN_BLOCKED:
+return False
+else:
+return True
+
+def check_savefile_create(*args):
+Check guest's managed save file be created
+
+(guestname) = args
+cmds = ls /var/lib/libvirt/qemu/save/%s % guestname + .save -lh
+logger.info(Execute cmd  %s % cmds)
+(status, output) = utils.exec_cmd(cmds, shell=True)
+if status != 0:
+logger.error(No managed save file)
+return False
+else :
+logger.info(managed save file exists)
+return True
+
+def compare_cachedfile(cachebefore, cacheafter):
+Compare cached value before managed save and its value after
+managed save 
+
+diff = cacheafter - cachebefore
+logger.info(diff is %s  % diff)
+percent = math.fabs(diff)/cachebefore
+logger.info(diff percent is %s  % percent)
+if math.fabs(diff)/cachebefore  0.05:


We get the variable 'percent', we can use it here instead 
of computing it twice.




+return True
+else:
+return False
+
+def get_cachevalue():
+Get the file system cached value 
+
+cmds = head -n4 /proc/meminfo|grep Cached|awk '{print $2}'
+(status, output) = utils.exec_cmd(cmds, shell=True)
+if status != 0:
+logger.error(failed to run cmd line to get cache)
+return 1
+else:
+logger.debug(output[0])
+cachevalue= int(output[0])
+return cachevalue
+
+def managedsave(params):
+Managed save a running domain
+
+global logger
+logger = params['logger']
+guestname = params['guestname']
+flags = params ['flags']
+#Save given flags to 

[libvirt] [PATCH 3/3] qemu: Add QEMU version computation to QMP probing

[Viktor Mihajlovski]

With QMP capability probing, the version was not set.
virsh version returns:
...
Cannot extract running QEMU hypervisor version

This is fixed by computing caps-version from QMP major,
minor, micro values.

Signed-off-by: Viktor Mihajlovski mihaj...@linux.vnet.ibm.com
---
 src/qemu/qemu_capabilities.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index a76d108..d92947f 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2377,6 +2377,7 @@ qemuCapsInitQMP(qemuCapsPtr caps,
 goto cleanup;
 }
 
+caps-version = major * 100 + minor * 1000 + micro;
 caps-usedQMP = true;
 
 qemuCapsInitQMPBasic(caps);
-- 
1.7.12.4

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

[libvirt] [PATCH 1/3] qemu: Wait for monitor socket even without pid

[Viktor Mihajlovski]

If qemuMonitorOpenUnix is called without a related pid, i.e. for
QMP probing, a connect failure can happen as the result of a race.
Without a pid there is no retry and thus we give up to early.
This changes the code to retry if no pid is supplied.

Signed-off-by: Viktor Mihajlovski mihaj...@linux.vnet.ibm.com
---
 src/qemu/qemu_monitor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index cbde2b1..fe8424f 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -283,7 +283,7 @@ qemuMonitorOpenUnix(const char *monitor, pid_t cpid)
 break;
 
 if ((errno == ENOENT || errno == ECONNREFUSED) 
-cpid  virProcessKill(cpid, 0) == 0) {
+(!cpid || virProcessKill(cpid, 0) == 0)) {
 /* ENOENT   : Socket may not have shown up yet
  * ECONNREFUSED : Leftover socket hasn't been removed yet */
 continue;
-- 
1.7.12.4

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

[libvirt] [PATCH 0/3] qemu: QMP Capability Probing Fixes

[Viktor Mihajlovski]

QMP Capability probing will fail if the QEMU process cannot create the
monitor socket file in /var/lib/libvirt/qemu. This is the case if the 
configured QEMU user is not root, but QEMU is run under root to perfom
the probing.
The suggested solution is to run QEMU as qemu user for probing as well.

As it happens, this developed into a mini-series: it was necessary
to let libvirt handle the pid file as this is stored in root-owned
directory /var/run/libvirt/qemu. This prompted a race condition opening
a socket. Last but not least caps-version was not filled with QMP
probing. 

Viktor Mihajlovski (3):
  qemu: Wait for monitor socket even without pid
  qemu: Fix QMP Capabability Probing Failure
  qemu: Add QEMU version computation to QMP probing

 src/qemu/qemu_capabilities.c | 89 ++--
 src/qemu/qemu_capabilities.h |  7 +++-
 src/qemu/qemu_driver.c   |  4 +-
 src/qemu/qemu_monitor.c  |  2 +-
 4 files changed, 78 insertions(+), 24 deletions(-)

-- 
1.7.12.4

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

[libvirt] [PATCH 2/3] qemu: Fix QMP Capabability Probing Failure

[Viktor Mihajlovski]

QMP Capability probing will fail if QEMU cannot bind to the
QMP monitor socket in the qemu_driver-libDir directory.
That's because the child process is stripped of all
capabilities and this directory is chown'ed to the configured
QEMU user/group (normally qemu:qemu) by the QEMU driver.

To prevent this from happening, the driver startup will now pass
the QEMU uid and gid down to the capability probing code.
All capability probing invocations of QEMU will be run with
the configured QEMU uid instead of libvirtd's.

Furter, the pid file handling is moved to libvirt, as QEMU
cannot write to the qemu_driver-runDir (root:root). This also
means that the libvirt daemonizing must be used.

Signed-off-by: Viktor Mihajlovski mihaj...@linux.vnet.ibm.com
---
 src/qemu/qemu_capabilities.c | 88 ++--
 src/qemu/qemu_capabilities.h |  7 +++-
 src/qemu/qemu_driver.c   |  4 +-
 3 files changed, 76 insertions(+), 23 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 6ce2638..a76d108 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -223,6 +223,8 @@ struct _qemuCapsCache {
 virHashTablePtr binaries;
 char *libDir;
 char *runDir;
+uid_t runUid;
+gid_t runGid;
 };
 
 
@@ -241,9 +243,37 @@ static int qemuCapsOnceInit(void)
 
 VIR_ONCE_GLOBAL_INIT(qemuCaps)
 
+struct _qemuCapsHookData {
+uid_t runUid;
+gid_t runGid;
+};
+typedef struct _qemuCapsHookData qemuCapsHookData;
+typedef qemuCapsHookData *qemuCapsHookDataPtr;
+
+static int qemuCapsHook(void * data)
+{
+int ret;
+qemuCapsHookDataPtr hookData = data;
+
+if (!hookData) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(QEMU uid:gid not specified by caller));
+ret = -1;
+goto cleanup;
+}
+
+VIR_DEBUG(Switch QEMU uid:gid to %d:%d,
+  hookData-runUid, hookData-runGid);
+ret = virSetUIDGID(hookData-runUid, hookData-runGid);
+
+cleanup:
+return ret;
+}
+
 static virCommandPtr
 qemuCapsProbeCommand(const char *qemu,
- qemuCapsPtr caps)
+ qemuCapsPtr caps,
+ qemuCapsHookDataPtr hookData)
 {
 virCommandPtr cmd = virCommandNew(qemu);
 
@@ -256,6 +286,7 @@ qemuCapsProbeCommand(const char *qemu,
 
 virCommandAddEnvPassCommon(cmd);
 virCommandClearCaps(cmd);
+virCommandSetPreExecHook(cmd, qemuCapsHook, hookData);
 
 return cmd;
 }
@@ -342,7 +373,7 @@ no_memory:
 }
 
 static int
-qemuCapsProbeMachineTypes(qemuCapsPtr caps)
+qemuCapsProbeMachineTypes(qemuCapsPtr caps, qemuCapsHookDataPtr hookData)
 {
 char *output;
 int ret = -1;
@@ -359,7 +390,7 @@ qemuCapsProbeMachineTypes(qemuCapsPtr caps)
 return -1;
 }
 
-cmd = qemuCapsProbeCommand(caps-binary, caps);
+cmd = qemuCapsProbeCommand(caps-binary, caps, hookData);
 virCommandAddArgList(cmd, -M, ?, NULL);
 virCommandSetOutputBuffer(cmd, output);
 
@@ -498,7 +529,7 @@ cleanup:
 }
 
 static int
-qemuCapsProbeCPUModels(qemuCapsPtr caps)
+qemuCapsProbeCPUModels(qemuCapsPtr caps, qemuCapsHookDataPtr hookData)
 {
 char *output = NULL;
 int ret = -1;
@@ -516,7 +547,7 @@ qemuCapsProbeCPUModels(qemuCapsPtr caps)
 return 0;
 }
 
-cmd = qemuCapsProbeCommand(caps-binary, caps);
+cmd = qemuCapsProbeCommand(caps-binary, caps, hookData);
 virCommandAddArgList(cmd, -cpu, ?, NULL);
 virCommandSetOutputBuffer(cmd, output);
 
@@ -1530,7 +1561,8 @@ qemuCapsParseDeviceStr(qemuCapsPtr caps, const char *str)
 
 static int
 qemuCapsExtractDeviceStr(const char *qemu,
- qemuCapsPtr caps)
+ qemuCapsPtr caps,
+ qemuCapsHookDataPtr hookData)
 {
 char *output = NULL;
 virCommandPtr cmd;
@@ -1544,7 +1576,7 @@ qemuCapsExtractDeviceStr(const char *qemu,
  * understand '-device name,?', and always exits with status 1 for
  * the simpler '-device ?', so this function is really only useful
  * if -help includes device driver,?.  */
-cmd = qemuCapsProbeCommand(qemu, caps);
+cmd = qemuCapsProbeCommand(qemu, caps, hookData);
 virCommandAddArgList(cmd,
  -device, ?,
  -device, pci-assign,?,
@@ -2094,7 +2126,7 @@ int qemuCapsProbeQMP(qemuCapsPtr caps,
 #define QEMU_SYSTEM_PREFIX qemu-system-
 
 static int
-qemuCapsInitHelp(qemuCapsPtr caps)
+qemuCapsInitHelp(qemuCapsPtr caps, uid_t runUid, gid_t runGid)
 {
 virCommandPtr cmd = NULL;
 unsigned int is_kvm;
@@ -2102,6 +2134,7 @@ qemuCapsInitHelp(qemuCapsPtr caps)
 int ret = -1;
 const char *tmp;
 struct utsname ut;
+qemuCapsHookData hookData;
 
 VIR_DEBUG(caps=%p, caps);
 
@@ -2123,7 +2156,9 @@ qemuCapsInitHelp(qemuCapsPtr caps)
 goto cleanup;
 }
 
-cmd = qemuCapsProbeCommand(caps-binary, NULL);
+hookData.runUid = runUid;
+hookData.runGid = runGid;
+cmd = 

[libvirt] [PATCH] lxc: Don't crash if no security driver is specified in libvirt_lxc

[Peter Krempa]

When no security driver is specified libvirt_lxc segfaults as a debug
message tries to access security labels for the container that are not
present.

This problem was introduced in commit 6c3cf57d6cb27cf10064baf8cca0f39.
---
 src/lxc/lxc_controller.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index ea5bc58..16cda9d 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -1612,11 +1612,15 @@ int main(int argc, char *argv[])
 false, false, false)))
 goto cleanup;

-VIR_DEBUG(Security model %s type %s label %s imagelabel %s,
-  NULLSTR(ctrl-def-seclabels[0]-model),
-  virDomainSeclabelTypeToString(ctrl-def-seclabels[0]-type),
-  NULLSTR(ctrl-def-seclabels[0]-label),
-  NULLSTR(ctrl-def-seclabels[0]-imagelabel));
+if (ctrl-def-seclabels) {
+VIR_DEBUG(Security model %s type %s label %s imagelabel %s,
+  NULLSTR(ctrl-def-seclabels[0]-model),
+  virDomainSeclabelTypeToString(ctrl-def-seclabels[0]-type),
+  NULLSTR(ctrl-def-seclabels[0]-label),
+  NULLSTR(ctrl-def-seclabels[0]-imagelabel));
+} else {
+VIR_DEBUG(Security model not initialized);
+}

 ctrl-veths = veths;
 ctrl-nveths = nveths;
-- 
1.8.0

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

Re: [libvirt] [PATCH] lxc: Avoid segfault of libvirt_lxc helper on early cleanup paths

[Jiri Denemark]

On Mon, Nov 26, 2012 at 12:18:17 +0100, Peter Krempa wrote:
 Early jumps to the cleanup label caused a crash of the libvirt_lxc
 container helper as the cleanup section called
 virLXCControllerDeleteInterfaces(ctrl) without checking the ctrl argument
 for NULL. The argument was de-referenced soon after.
 
 $ /usr/libexec/libvirt_lxc
 /usr/libexec/libvirt_lxc: missing --name argument for configuration
 Segmentation fault
 ---
  src/lxc/lxc_controller.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

ACK

Jirka

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

Re: [libvirt] [PATCH] lxc: Don't crash if no security driver is specified in libvirt_lxc

[Jiri Denemark]

On Mon, Nov 26, 2012 at 15:22:25 +0100, Peter Krempa wrote:
 When no security driver is specified libvirt_lxc segfaults as a debug
 message tries to access security labels for the container that are not
 present.
 
 This problem was introduced in commit 6c3cf57d6cb27cf10064baf8cca0f39.
 ---
  src/lxc/lxc_controller.c | 14 +-
  1 file changed, 9 insertions(+), 5 deletions(-)

ACK

Jirka

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

Re: [libvirt] [PATCH] lxc: Don't crash if no security driver is specified in libvirt_lxc

[Peter Krempa]

On 11/26/12 15:48, Jiri Denemark wrote:

On Mon, Nov 26, 2012 at 15:22:25 +0100, Peter Krempa wrote:

When no security driver is specified libvirt_lxc segfaults as a debug
message tries to access security labels for the container that are not
present.

This problem was introduced in commit 6c3cf57d6cb27cf10064baf8cca0f39.
---
  src/lxc/lxc_controller.c | 14 +-
  1 file changed, 9 insertions(+), 5 deletions(-)


ACK

Jirka



Pushed, thanks.

Peter

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

Re: [libvirt] [PATCH] lxc: Avoid segfault of libvirt_lxc helper on early cleanup paths

[Peter Krempa]

On 11/26/12 15:35, Jiri Denemark wrote:

On Mon, Nov 26, 2012 at 12:18:17 +0100, Peter Krempa wrote:

Early jumps to the cleanup label caused a crash of the libvirt_lxc
container helper as the cleanup section called
virLXCControllerDeleteInterfaces(ctrl) without checking the ctrl argument
for NULL. The argument was de-referenced soon after.

$ /usr/libexec/libvirt_lxc
/usr/libexec/libvirt_lxc: missing --name argument for configuration
Segmentation fault
---
  src/lxc/lxc_controller.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)


ACK

Jirka


Pushed, thanks.

Peter

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

Re: [libvirt] [PATCH v4] qemu: Allow the user to specify vendor and product for disk

[Osier Yang]

ping

On 2012年11月22日 15:55, Osier Yang wrote:

QEMU supports setting vendor and product strings for disk since
1.2.0 (only scsi-disk, scsi-hd, scsi-cd support it), this patch
exposes it with new XML elementsvendor  andproduct  of disk
device.

v3 - v4:

   * Per Paolo's feedback, allows all printable chars.
---
  docs/formatdomain.html.in  |   11 +
  docs/schemas/domaincommon.rng  |   14 ++
  src/conf/domain_conf.c |   44 
  src/conf/domain_conf.h |2 +
  src/libvirt_private.syms   |1 +
  src/qemu/qemu_command.c|   29 +
  src/util/util.c|   12 +
  src/util/util.h|1 +
  ...qemuxml2argv-disk-scsi-disk-vpd-build-error.xml |   35 
  .../qemuxml2argv-disk-scsi-disk-vpd.args   |   13 ++
  .../qemuxml2argv-disk-scsi-disk-vpd.xml|   38 +
  tests/qemuxml2argvtest.c   |8 
  tests/qemuxml2xmltest.c|2 +
  13 files changed, 210 insertions(+), 0 deletions(-)
  create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd-build-error.xml
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.args
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-scsi-disk-vpd.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 6a3b976..903c069 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1657,6 +1657,17 @@
  of 16 hexadecimal digits.
  span class='since'Since 0.10.1/span
/dd
+dtcodevendor/code/dt
+ddIf present, this element specifies the vendor of a virtual hard
+disk or CD-ROM device. It must not be longer than 8 printable
+characters.
+span class='since'Since 1.0.1/span
+/dd
+dtcodeproduct/code/dt
+ddIf present, this element specifies the product of a virtual hard
+disk or CD-ROM device. It must not be longer than 16 printable
+span class='since'Since 1.0.1/span
+/dd
dtcodehost/code/dt
ddThecodehost/code  element has two attributes name and port,
  which specify the hostname and the port number. The meaning of this
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 02ad477..19bf597 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -905,6 +905,20 @@
ref name=wwn/
  /element
/optional
+optional
+element name=vendor
+data type=string
+param name=pattern[x20-x7E]{0,8}/param
+/data
+/element
+/optional
+optional
+element name=product
+data type=string
+param name=pattern[x20-x7E]{0,16}/param
+/data
+/element
+/optional
  /interleave
/define
define name=snapshot
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 047c4fc..e975f74 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -979,6 +979,8 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
  VIR_FREE(def-mirror);
  VIR_FREE(def-auth.username);
  VIR_FREE(def-wwn);
+VIR_FREE(def-vendor);
+VIR_FREE(def-product);
  if (def-auth.secretType == VIR_DOMAIN_DISK_SECRET_TYPE_USAGE)
  VIR_FREE(def-auth.secret.usage);
  virStorageEncryptionFree(def-encryption);
@@ -3490,6 +3492,8 @@ cleanup:
  goto cleanup;
  }

+#define VENDOR_LEN  8
+#define PRODUCT_LEN 16

  /* Parse the XML definition for a disk
   * @param node XML nodeset to parse for disk definition
@@ -3542,6 +3546,8 @@ virDomainDiskDefParseXML(virCapsPtr caps,
  char *logical_block_size = NULL;
  char *physical_block_size = NULL;
  char *wwn = NULL;
+char *vendor = NULL;
+char *product = NULL;

  if (VIR_ALLOC(def)  0) {
  virReportOOMError();
@@ -3880,6 +3886,36 @@ virDomainDiskDefParseXML(virCapsPtr caps,

  if (!virValidateWWN(wwn))
  goto error;
+} else if (!vendor
+   xmlStrEqual(cur-name, BAD_CAST vendor)) {
+vendor = (char *)xmlNodeGetContent(cur);
+
+if (strlen(vendor)  VENDOR_LEN) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(disk vendor is more than 8 characters));
+goto error;
+}
+
+if (!virStrIsPrint(vendor)) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(disk vendor is not printable string));
+goto error;
+}
+} else if (!product
+   xmlStrEqual(cur-name, BAD_CAST product)) {
+product = (char *)xmlNodeGetContent(cur);
+
+if (strlen(vendor)  PRODUCT_LEN) {
+virReportError(VIR_ERR_XML_ERROR, 

Re: [libvirt] [PATCH 1/2] node_memory: Do not fail if there is parameter unsupported

[Osier Yang]

ping

On 2012年11月22日 11:14, Osier Yang wrote:

It makes no sense to fail the whole command if there is parameter
unsupported by the kernel. This patch fixes it by ignoring the
unsupported parameter for getMemoryParameters, and ignoring the
unsupported parameter for setMemoryParameters too if there are
more than one parameters to set, otherwise error out.
---
  src/libvirt.c  |   12 +++--
  src/nodeinfo.c |  119 ++--
  2 files changed, 79 insertions(+), 52 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c
index 4af6089..93ec068 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -6746,10 +6746,10 @@ error:
   * @nparams: pointer to number of memory parameters; input and output
   * @flags: extra flags; not used yet, so callers should always pass 0
   *
- * Get all node memory parameters.  On input, @nparams gives the size
- * of the @params array; on output, @nparams gives how many slots were
- * filled with parameter information, which might be less but will
- * not exceed the input value.
+ * Get all node memory parameters (parameter unsupported by OS will be
+ * ignored).  On input, @nparams gives the size of the @params array;
+ * on output, @nparams gives how many slots were filled with parameter
+ * information, which might be less but will not exceed the input value.
   *
   * As a special case, calling with @params as NULL and @nparams as 0 on
   * input will cause @nparams on output to contain the number of parameters
@@ -6811,7 +6811,9 @@ error:
   *   value nparams of virDomainGetSchedulerType)
   * @flags: extra flags; not used yet, so callers should always pass 0
   *
- * Change all or a subset of the node memory tunables.
+ * Change all or a subset of the node memory tunables. Tunable
+ * unsupported by OS wll be ignored if @nparams is not 1, otherwise
+ * the function fails.
   *
   * Note that it's not recommended to use this function while the
   * outside tuning program is running (such as ksmtuned under Linux),
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 75d6524..718a3a4 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -1058,21 +1058,13 @@ nodeGetCPUBitmap(int *max_id ATTRIBUTE_UNUSED)

  #ifdef __linux__
  static int
-nodeSetMemoryParameterValue(const char *field,
+nodeSetMemoryParameterValue(const char *path,
  virTypedParameterPtr param)
  {
-char *path = NULL;
  char *strval = NULL;
  int ret = -1;
  int rc = -1;

-if (virAsprintf(path, %s/%s,
-SYSFS_MEMORY_SHARED_PATH, field)  0) {
-virReportOOMError();
-ret = -2;
-goto cleanup;
-}
-
  if (virAsprintf(strval, %u, param-value.ui) == -1) {
  virReportOOMError();
  ret = -2;
@@ -1080,13 +1072,12 @@ nodeSetMemoryParameterValue(const char *field,
  }

  if ((rc = virFileWriteStr(path, strval, 0))  0) {
-virReportSystemError(-rc, _(failed to set %s), field);
+virReportSystemError(-rc, _(failed to set %s), param-field);
  goto cleanup;
  }

  ret = 0;
  cleanup:
-VIR_FREE(path);
  VIR_FREE(strval);
  return ret;
  }
@@ -1101,7 +1092,9 @@ nodeSetMemoryParameters(virConnectPtr conn 
ATTRIBUTE_UNUSED,
  virCheckFlags(0, -1);

  #ifdef __linux__
-int ret = 0;
+char *path = NULL;
+int ret = -1;
+int rc;
  int i;

  if (virTypedParameterArrayValidate(params, nparams,
@@ -1117,30 +1110,40 @@ nodeSetMemoryParameters(virConnectPtr conn 
ATTRIBUTE_UNUSED,
  for (i = 0; i  nparams; i++) {
  virTypedParameterPtr param =params[i];

-if (STREQ(param-field,
-  VIR_NODE_MEMORY_SHARED_PAGES_TO_SCAN)) {
-ret = nodeSetMemoryParameterValue(pages_to_scan, param);
+char *field = strchr(param-field, '_');
+field++;
+if (virAsprintf(path, %s/%s,
+SYSFS_MEMORY_SHARED_PATH, field)  0) {
+virReportOOMError();
+goto cleanup;
+}

-/* Out of memory */
-if (ret == -2)
-return -1;
-} else if (STREQ(param-field,
- VIR_NODE_MEMORY_SHARED_SLEEP_MILLISECS)) {
-ret = nodeSetMemoryParameterValue(sleep_millisecs, param);
+/* Ignore the unsupported the param field if there is other
+ * parameter to set, otherwise error out.
+ */
+if (!virFileExists(path)) {
+if (nparams == 1) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   _(Parameter '%s' is not supported by 
+ this kernel), param-field);
+goto cleanup;
+} else {
+VIR_WARN(Parameter '%s' is not supported by this kernel,
+ param-field);
+continue;
+}
+}

-/* Out of memory */
-if (ret == -2)
-

Re: [libvirt] Proposal: no dnsmasq (no dhcp and no dns) and no radvd option

[Gene Czarcinski]

On 11/25/2012 05:08 PM, Laine Stump wrote:

On 11/25/2012 08:25 AM, Gene Czarcinski wrote:

On 11/24/2012 04:19 PM, Laine Stump wrote:

On 11/23/2012 02:18 PM, Gene Czarcinski wrote:

On 11/20/2012 05:29 PM, Laine Stump wrote:

On 11/20/2012 02:36 PM, Gene Czarcinski wrote:

Laine mentioned something yesterday that got me to thinking: being
able to specify that dnsmasq is not to be started for an interface.

Let me expand that by saying that libvirt would not start dnsmasq
for
either dns or dhcp and also would not start radvd.  However, the
IPv4
and IPv6 gateway addresses would be defined on the virtual network
interface and the usual iptables and ip6tables rules would be in

force.

This would allow a user to configure dnsmasq to meet any user
desires
or use something completely different instead of dnsmasq.

Questions:  Useful?  Worth the time and effort?

That was already determined before I mentioned it to you - it's been
requested several times, and I've told some people it was going to
happen, although didn't say when:-).


OK, color this almost done (doc and schemas need updating).

Three new parameters are added:
network disableDnsmasq='yes'  logDnsQueries='yes'
logDhcp='yes' 

Don't refer to dnsmasq specifically in the XML unless it's something
that is unique to dnsmasq (and then we should seriously consider  if
we're thinking about it the right way before adding it). The intent is
that the XML should describe the desired configuration in a manner high
level enough that a new driver using different backend components would
be able to use the same XML attributes and arrive at similar
functionality. For example, there is active work right now to support
libvirt's server side on FreeBSD, and they may decide that they want to
implement their networks using different implementations of DNS and dhcp
servers (they will already need to do something different in place of
Linux host brdges and iptables, so it will have to be a completely new
driver anyway).

Obviously there are cases where something specific to the backend needs
to be in the XML; for example the virtio-net drivers can specify driver
name='qemu' ... and in that case can have an option ioeventfd and
event_idx options which would probably be meaningless to any netdev
driver for another hypervisor, but we aim to keep that to a minimum, and
use it only when the characteristic described really is unique to
particular backend).

(At any rate, it seems reasonable to me that someone may want us to
provide a dhcp server but no dns server. Yes, this also implies that we
should have a way in the config to feed in the address of an alternative
DNS server (--dhcp-option=option:dns-server,x.x.x.x and
--dhcp-option=option6:dnsserver,::) to send to dhcp clients
when they've disabled dnsmasq's own dns server (in case you hadn't found
it in the manpage yet - you can run dnsmasq with no dns server by adding
--port=0 to the commandline). As soon as I hit send on this, I'm going
to type up a short mail asking for comments on how to add support for
any standard dhcp option.

As for the other two - any reason why you used logDnsQueries instead
of logDns? Also, do you think we might want the ability to specify a
level in the future? (I guess if we did, that could coexist with
yes|no).

And while we're on the topic of logging: one thing that's bothered me a
lot, and we've had other people complain about it as well, is that every
single DHCP lease renewal results in a message to /var/log/messages, and
there doesn't seem to be any way to suppress it. Is that correct?

OK, I had not realized that it was possible to not have dns while
continuing to do dhcp.  So, it now becomes disableDns=yes which
results in --port=0 being passed to dnsmasq.  For completeness, I
suggest that disableDhcp=yes and disableRA=yes be added.

disableDhcp already exists - you just don't have a dhcp section (and
if you do have a dhcp section, it seems very illogical that you would
want dhcp disabled).
Yes, you are correct.  I was adding that for the symmetry and 
completeness ;)


disableRA may be a valid concern, since that's on by default if there
are any IPv6 addresses.


However, to pull the discussion back in - I had earlier suggested *not*
having an attribute called disableXXX because it starts to get
confusing - you're saying Yes, I don't want this feature.. It's
simpler to understand *No*, I *don't* want this feature. And I'd also
suggested making it an option of the already existing dns element (see
below).
I understand your point and will use an enable user interface with the 
default being that, in general, things are enabled unless enableXX='no' 
is specified.



I have changed my mind and will make it logDns=, logDhcp= and (again
for completeness) logRA= ... this last one may require a small dnsmasq
update.

I like dns log='xxx'.../ (edit: I may have changed my opinion on
this - see the end of the message)
And where would you suggest logDhcp adding logDhcp?  

Re: [libvirt] Proposal: no dnsmasq (no dhcp and no dns) and no radvd option

[Gene Czarcinski]

On 11/26/2012 10:40 AM, Gene Czarcinski wrote:

OK, enable= and log= will go in the dns element.

For, enableRA, it will go in any ip family='ipv6. If enableRA='yes' 
and if dhcp is specified for that element, the stateful RA will be 
configured.  If enableRA='yes' (the default) and no dhcp is specified, 
then stateless RA is configured.  If enableRA='no', no RA is 
configured for that subnet.  If all ipv6 specifications have 
enableRA='no', then nether radvd will be started nor will dnsmasq be 
configured for RA.


If any IPv4 or IPv6 DHCP specification which includes log='yes', then 
log-dhcp will be specified for the interface.


When (hopefully not if) dnsmasq is changed to include log-ra to enable 
messages about RA sent to syslog and log-ra is added to --help, then 
any IP family='ipv6' logRA='yes' ... / will enable it for all IPv6 
addresses on that interface.


If dns is disabled and there is no dhcp specification but RA is not 
disabled on all IPv6 specifications, then ??


It is not clear to me that dnsmasq could handle state-less RA if there 
is no dns and no dhcp specified.  In fact, the way state-less RA is 
specified is with dhcp-range=ipv6-address,ra-only ... more testing! 

On other thing ...

When a network specification is saved back to an xml file, should the 
default values of these new parameters be saved.  I believe the answer 
is no.  For example, ip family='ipv4' is the default but is not saved 
as such.


Gene

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

Re: [libvirt] Proposal: no dnsmasq (no dhcp and no dns) and no radvd option

[Laine Stump]

On 11/26/2012 10:56 AM, Gene Czarcinski wrote:
 One other thing ...

 When a network specification is saved back to an xml file, should the
 default values of these new parameters be saved.  I believe the answer
 is no.  For example, ip family='ipv4' is the default but is not saved
 as such.

Generally the *Format() functions omit attributes/elements if they are
set to the default value.

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

Re: [libvirt] Proposal: no dnsmasq (no dhcp and no dns) and no radvd option

[Gene Czarcinski]

On 11/26/2012 10:40 AM, Gene Czarcinski wrote:
I understand that you can define multiple IPv4 and multiple IPv6 
gateway addresses on a network interface but only one IPv4 DHCP and 
one IPv6 DHCP.  I can see the need for both IPv4 and IPv6 protocols on 
a single network fabric but I am not sure how many real network 
fabrics have multiple subnetworks on them.  Yes, it could be done 
but I am not certain why you would do that (and I am also sure that 
someone has a very valid reason for doing that). 

Oops!  There may be a problem here with radvd!.

I have difficulty in understanding why one would define multiple IPv6 
(or even IPv4) subnetworks on a single interface.  Well, I guess the 
radvd authors did also: the AdvManagedFlag on/off applies to the entire 
interface and no a specific network.


I am verifying this but there is a chance that dsnmasq could support 
both for different subnetworks.


I guess that dnsmasq could be used to support one and radvd used to 
support the other but ???


I believe this may need more discussion from others.  I would like to 
have someone other than the two of us chime in on this.


Gene

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

[libvirt] [PATCHv2 1/2] S390: Add SCLP console front end support

[Viktor Mihajlovski]

From: J.B. Joret j...@linux.vnet.ibm.com

The SCLP console is the native console type for s390 and is preferred
over the virtio console as it doesn't require special drivers and
is more efficient. Recent versions of QEMU come with SCLP support
which is hereby enabled.

The new target types 'sclp' and 'sclplm' can be used to specify a
SCLP console. Adding documentation, domain schema and XML processing
support.

Signed-off-by: J.B. Joret j...@linux.vnet.ibm.com
Signed-off-by: Viktor Mihajlovski mihaj...@linux.vnet.ibm.com
---
 docs/formatdomain.html.in | 19 ++-
 docs/schemas/domaincommon.rng |  2 ++
 src/conf/domain_conf.c|  4 +++-
 src/conf/domain_conf.h|  2 ++
 4 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 6a3b976..6cca088 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3602,7 +3602,7 @@ qemu-kvm -net nic,model=? /dev/null
 /p
 
 ul
-  liIf no codetargetType/code attribue is set, then the default
+  liIf no codetargetType/code attribute is set, then the default
 device type is according to the hypervisor's rules. The default
 type will be added when re-querying the XML fed into libvirt.
 For fully virtualized guests, the default device type will usually
@@ -3616,6 +3616,12 @@ qemu-kvm -net nic,model=? /dev/null
   liOnly the first codeconsole/code element may use a 
codetargetType/code
 of codeserial/code. Secondary consoles must all be paravirtualized.
   /li
+  liOn s390, the codeconsole/code element may use a
+codetargetType/code of codesclp/code or codesclplm/code
+(line mode). SCLP is the native console type for s390. There's no
+controller associated to SCLP consoles.
+span class=sinceSince 1.0.1/span
+  /li
 /ul
 
 p
@@ -3641,6 +3647,17 @@ qemu-kvm -net nic,model=? /dev/null
   lt;/devicesgt;
   .../pre
 
+pre
+  ...
+  lt;devicesgt;
+lt;!-- KVM s390 sclp console --gt;
+lt;console type='pty'gt;
+  lt;source path='/dev/pts/1'/gt;
+  lt;target type='sclp' port='0'/gt;
+lt;/consolegt;
+  lt;/devicesgt;
+  .../pre
+
 p
   If the console is presented as a serial port, the codetarget/code
   element has the same attributes as for a serial port. There is usually
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 02ad477..751d307 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2406,6 +2406,8 @@
 valuevirtio/value
 valuelxc/value
 valueopenvz/value
+valuesclp/value
+valuesclplm/value
   /choice
 /attribute
   /define
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ed8b53f..845919b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -344,7 +344,9 @@ VIR_ENUM_IMPL(virDomainChrConsoleTarget,
   uml,
   virtio,
   lxc,
-  openvz)
+  openvz,
+  sclp,
+  sclplm)
 
 VIR_ENUM_IMPL(virDomainChrDevice, VIR_DOMAIN_CHR_DEVICE_TYPE_LAST,
   parallel,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index c3e8c16..ab38bd0 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -892,6 +892,8 @@ enum virDomainChrConsoleTargetType {
 VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO,
 VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LXC,
 VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_OPENVZ,
+VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLP,
+VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLPLM,
 
 VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST
 };
-- 
1.7.12.4

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

[libvirt] [PATCHv2 2/2] S390: Enable SCLP Console in QEMU driver

[Viktor Mihajlovski]

From: J.B. Joret j...@linux.vnet.ibm.com

This is the QEMU backend code for the SCLP console support.
It includes SCLP capability detection, QEMU command line generation
and a test case.

Signed-off-by: J.B. Joret j...@linux.vnet.ibm.com
Signed-off-by: Viktor Mihajlovski mihaj...@linux.vnet.ibm.com
---
 src/qemu/qemu_capabilities.c   |  3 ++
 src/qemu/qemu_capabilities.h   |  1 +
 src/qemu/qemu_command.c| 59 ++
 .../qemuxml2argv-console-sclp.args |  8 +++
 .../qemuxml2argvdata/qemuxml2argv-console-sclp.xml | 24 +
 tests/qemuxml2argvtest.c   |  3 ++
 6 files changed, 98 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-console-sclp.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-console-sclp.xml

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 6ce2638..c08fa1e 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -193,6 +193,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
   drive-mirror, /* 115 */
   usb-redir.bootindex,
   usb-host.bootindex,
+  s390-sclp,
+
 );
 
 struct _qemuCaps {
@@ -1279,6 +1281,7 @@ struct qemuCapsStringFlags qemuCapsObjectTypes[] = {
 { usb-hub, QEMU_CAPS_USB_HUB },
 { ich9-ahci, QEMU_CAPS_ICH9_AHCI },
 { virtio-blk-s390, QEMU_CAPS_VIRTIO_S390 },
+{ sclpconsole, QEMU_CAPS_SCLP_S390 },
 { lsi53c895a, QEMU_CAPS_SCSI_LSI },
 { virtio-scsi-pci, QEMU_CAPS_VIRTIO_SCSI_PCI },
 { spicevmc, QEMU_CAPS_DEVICE_SPICEVMC },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 751b3ec..b05db28 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -155,6 +155,7 @@ enum qemuCapsFlags {
 QEMU_CAPS_DRIVE_MIRROR   = 115, /* drive-mirror monitor command */
 QEMU_CAPS_USB_REDIR_BOOTINDEX = 116, /* usb-redir.bootindex */
 QEMU_CAPS_USB_HOST_BOOTINDEX = 117, /* usb-host.bootindex */
+QEMU_CAPS_SCLP_S390  = 118, /* -device sclp* */
 
 QEMU_CAPS_LAST,   /* this must always be the last item */
 };
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9c9a0ed..5849944 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3913,6 +3913,37 @@ error:
 return NULL;
 }
 
+static char *qemuBuildSclpDevStr(virDomainChrDefPtr dev)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+if (dev-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE) {
+switch (dev-targetType) {
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLP:
+virBufferAddLit(buf, sclpconsole);
+break;
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLPLM:
+virBufferAddLit(buf, sclplmconsole);
+break;
+}
+} else {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(Cannot use slcp with devices other than console));
+goto error;
+}
+virBufferAsprintf(buf, ,chardev=char%s,id=%s,
+  dev-info.alias, dev-info.alias);
+if (virBufferError(buf)) {
+virReportOOMError();
+goto error;
+}
+
+return virBufferContentAndReset(buf);
+
+error:
+virBufferFreeAndReset(buf);
+return NULL;
+}
+
 static char *qemuBuildSmbiosBiosStr(virSysinfoDefPtr def)
 {
 virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -6151,6 +6182,34 @@ qemuBuildCommandLine(virConnectPtr conn,
 char *devstr;
 
 switch (console-targetType) {
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLP:
+case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SCLPLM:
+if (!qemuCapsGet(caps, QEMU_CAPS_DEVICE)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(sclp console requires QEMU to support 
-device));
+goto error;
+}
+if (!qemuCapsGet(caps, QEMU_CAPS_SCLP_S390)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(sclp console requires QEMU to support 
s390-sclp));
+goto error;
+}
+
+virCommandAddArg(cmd, -chardev);
+if (!(devstr = qemuBuildChrChardevStr(console-source,
+  console-info.alias,
+  caps)))
+goto error;
+virCommandAddArg(cmd, devstr);
+VIR_FREE(devstr);
+
+virCommandAddArg(cmd, -device);
+if (!(devstr = qemuBuildSclpDevStr(console)))
+goto error;
+virCommandAddArg(cmd, devstr);
+VIR_FREE(devstr);
+break;
+
 case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO:
 if (!qemuCapsGet(caps, QEMU_CAPS_DEVICE)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,

[libvirt] [PATCHv2 0/2] S390: Adding support for SCLP Console

[Viktor Mihajlovski]

The S390 architecture comes with a native console type (SCLP
console) which is now also supported by current QEMU.

This series is enabling libvirt to configure S390 domains with SCLP
consoles.

The domain XML has to be extended for the new console target types
'sclp' and 'sclplm' (line mode = dumb).

As usual the QEMU driver must do capability probing in order to find
out whether SCLP is supported and format the QEMU command line
for the new console type.

V2 Changes:
 Rebased to current master, resolving conflicts.

J.B. Joret (2):
  S390: Add SCLP console front end support
  S390: Enable SCLP Console in QEMU driver

 docs/formatdomain.html.in  | 19 ++-
 docs/schemas/domaincommon.rng  |  2 +
 src/conf/domain_conf.c |  4 +-
 src/conf/domain_conf.h |  2 +
 src/qemu/qemu_capabilities.c   |  3 ++
 src/qemu/qemu_capabilities.h   |  1 +
 src/qemu/qemu_command.c| 59 ++
 .../qemuxml2argv-console-sclp.args |  8 +++
 .../qemuxml2argvdata/qemuxml2argv-console-sclp.xml | 24 +
 tests/qemuxml2argvtest.c   |  3 ++
 10 files changed, 123 insertions(+), 2 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-console-sclp.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-console-sclp.xml

-- 
1.7.12.4

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

Re: [libvirt] Proposal: no dnsmasq (no dhcp and no dns) and no radvd option

[Gene Czarcinski]

On 11/25/2012 05:08 PM, Laine Stump wrote:

(btw, you must be running with net.bridge.bridge-nf-call-iptables=1,
otherwise communications between guests (ipv4 and ipv6) would work just
fine with no extra rules)

Do you know what sets this?

On a system with no virtualization installed, 
net.bridge.bridge-nf-call-iptables=0 but with virtualization (and a lot 
of other stuff) installed, it is set to 1.  I found only a single 
reference within libvirt which tests to see if they are set to 1.


The values in /etc/sysctl.conf are all =0 so this must be related to 
something virtualization is doing (but not necessarily libvirt).


Regardless, they are set so the ip6tables rule to allow guest-to-guest 
commo is needed.


Gene

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

[libvirt] [PATCH] Add detailed error information to journal

[Miloslav Trmač]

When logging an error, don't throw away the detailed information.
Example record when using the journald output:

MESSAGE=Domain not found
PRIORITY=4
LIBVIRT_SOURCE=error
CODE_FILE=../../src/test/test_driver.c
CODE_LINE=1406
CODE_FUNC=testLookupDomainByName
DOMAIN=12
CODE=42

The format used in other output destinations (e.g. syslog, file) is
still unchanged.

The domain and code numbers are part of the libvirt ABI in
libvirt/virterror.h; therefore log processing tools can rely on them,
unlike the text log string (which is translated depending on locale,
and may be modified for other reasons as well).

Alternatively, the domain and code fields could contain strings
instead of numbers, but it's not clear that it's worth it:
Advantages of numbers:
* the numbers are shorter
* the ABI guarantees that the numbers won't change
Disadvantages of strings:
* adding a ABI-stable string mapping for virErrorNumber would result
  in additional work each time a new error number is added
  (note that virErrorMsg cannot be used for this because it is
  translated)
* a change in the string mapping would be less likely to be noticed
The advantage of using strings is more readability, but note that the
msg field above already contains a readable description of the
error.

The inability to allocate memory imposes a fixed limit on the number
of metadata fields that can be supported by the journal; fields beyond
this limit are silently dropped (but the initial part of the message
sent).  This was implemented in the previous patch, here we just
increase the limit to 32 fields total.

Signed-off-by: Miloslav Trmač m...@redhat.com

---

This version drops logging the str[123] and int[12] fields.

The META_ADD_STRING macro is unused; it was left there just to
demonstrate the use of the API in case someone wanted to add more
error-specific information.
---
 src/util/logging.c   | 13 +++--
 src/util/virterror.c | 23 ++-
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/src/util/logging.c b/src/util/logging.c
index e8fed55..dbffaa1 100644
--- a/src/util/logging.c
+++ b/src/util/logging.c
@@ -1125,7 +1125,7 @@ virLogOutputToJournald(virLogSource source,
int linenr,
const char *funcname,
const char *timestamp ATTRIBUTE_UNUSED,
-   virLogMetadataPtr metadata ATTRIBUTE_UNUSED,
+   virLogMetadataPtr metadata,
unsigned int flags,
const char *rawstr,
const char *str ATTRIBUTE_UNUSED,
@@ -1145,7 +1145,7 @@ virLogOutputToJournald(virLogSource source,
  * and where unprivileged users can create files. */
 char path[] = /dev/shm/journal.XX;
 
-#  define NUM_FIELDS 6
+#  define NUM_FIELDS 32
 struct iovec iov[NUM_FIELDS * 5];
 char iov_bufs[NUM_FIELDS][JOURNAL_BUF_SIZE];
 struct journalState state;
@@ -1162,6 +1162,15 @@ virLogOutputToJournald(virLogSource source,
 journalAddString(state, CODE_FILE, filename);
 journalAddInt(state, CODE_LINE, linenr);
 journalAddString(state, CODE_FUNC, funcname);
+if (metadata != NULL) {
+while (metadata-key != NULL) {
+if (metadata-s != NULL)
+journalAddString(state, metadata-key, metadata-s);
+else
+journalAddInt(state, metadata-key, metadata-i);
+metadata++;
+}
+}
 
 memset(sa, 0, sizeof(sa));
 sa.sun_family = AF_UNIX;
diff --git a/src/util/virterror.c b/src/util/virterror.c
index 213188e..10b627f 100644
--- a/src/util/virterror.c
+++ b/src/util/virterror.c
@@ -619,6 +619,8 @@ virRaiseErrorFull(const char *filename ATTRIBUTE_UNUSED,
 virErrorPtr to;
 char *str;
 int priority;
+virLogMetadata meta[3];
+size_t i;
 
 /*
  * All errors are recorded in thread local storage
@@ -676,10 +678,29 @@ virRaiseErrorFull(const char *filename ATTRIBUTE_UNUSED,
 priority = virErrorLevelPriority(level);
 if (virErrorLogPriorityFilter)
 priority = virErrorLogPriorityFilter(to, priority);
+
+i = 0;
+#define META_ADD_STRING(KEY, VALUE) \
+do {\
+meta[i].key = (KEY);\
+meta[i].s = (VALUE);\
+i++;\
+} while (0)
+#define META_ADD_INT(KEY, VALUE)\
+do {\
+meta[i].key = (KEY);\
+meta[i].s = NULL;   \
+meta[i].i = (VALUE);\
+i++;\
+} while (0)
+
+META_ADD_INT(DOMAIN, domain);
+META_ADD_INT(CODE, code);
+meta[i].key = NULL;
 virLogMessage(virErrorLogPriorityFilter ? VIR_LOG_FROM_FILE : 
VIR_LOG_FROM_ERROR,
   

Re: [libvirt] [PATCH 1/3] util: capabilities detection for dnsmasq

[Laine Stump]

On 11/22/2012 12:55 AM, Doug Goldstein wrote:
 On Nov 21, 2012, at 8:55 PM, Laine Stump la...@laine.org wrote:

 In order to optionally take advantage of new features in dnsmasq when
 the host's version of dnsmasq supports them, but still be able to run
 on hosts that don't support the new features, we need to be able to
 detect the version of dnsmasq running on the host, and possibly
 determine from the help output what options are in this dnsmasq.

 This patch implements a greatly simplified version of the capabilities
 code we already have for qemu. A dnsmasqCaps device can be created and
 populated either from running a program on disk, reading a file with
 the concatenated output of dnsmasq --version; dnsmasq --help, or
 examining a buffer in memory that contains the concatenated output of
 those two commands. Simple functions to retrieve capabilities flags,
 the version number, and the path of the binary are also included.

 bridge_driver.c creates a single dnsmasqCaps object at driver startup,
 and disposes of it at driver shutdown. Any time it must be used, the
 dnsmasqCapsRefresh method is called - it checks the mtime of the
 binary, and re-runs the checks if the binary has changed.

 networkxml2argvtest.c creates 2 artificial dnsmasqCaps objects at
 startup - one restricted (doesn't support --bind-dynamic) and one
 full (does support --bind-dynamic). Some of the test cases use one
 and some the other, to make sure both code pathes are tested.
 ---
 src/libvirt_private.syms|   8 +-
 src/network/bridge_driver.c |  59 ++
 src/network/bridge_driver.h |   7 +-
 src/util/dnsmasq.c  | 260 
 
 src/util/dnsmasq.h  |  20 +++-
 tests/networkxml2argvtest.c |  57 ++
 6 files changed, 366 insertions(+), 45 deletions(-)

 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
 index 5a07139..24d2033 100644
 --- a/src/libvirt_private.syms
 +++ b/src/libvirt_private.syms
 @@ -244,13 +244,19 @@ virDevicePCIAddressParseXML;
 # dnsmasq.h
 dnsmasqAddDhcpHost;
 dnsmasqAddHost;
 +dnsmasqCapsGet;
 +dnsmasqCapsGetBinaryPath;
 +dnsmasqCapsGetVersion;
 +dnsmasqCapsNewFromBuffer;
 +dnsmasqCapsNewFromFile;
 +dnsmasqCapsNewFromBinary;
 +dnsmasqCapsRefresh;
 dnsmasqContextFree;
 dnsmasqContextNew;
 dnsmasqDelete;
 dnsmasqReload;
 dnsmasqSave;

 -
 # domain_audit.h
 virDomainAuditCgroup;
 virDomainAuditCgroupMajor;
 diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
 index c153d36..34923ea 100644
 --- a/src/network/bridge_driver.c
 +++ b/src/network/bridge_driver.c
 @@ -85,6 +85,7 @@ struct network_driver {
 char *networkConfigDir;
 char *networkAutostartDir;
 char *logDir;
 +dnsmasqCapsPtr dnsmasqCaps;
 };


 @@ -271,7 +272,8 @@ networkFindActiveConfigs(struct network_driver *driver) {
 char *radvdpidbase;

 ignore_value(virPidFileReadIfAlive(NETWORK_PID_DIR, 
 obj-def-name,
 -   obj-dnsmasqPid, 
 DNSMASQ));
 +   obj-dnsmasqPid,
 +   
 dnsmasqCapsGetBinaryPath(driver-dnsmasqCaps)));

 if (!(radvdpidbase = 
 networkRadvdPidfileBasename(obj-def-name))) {
 virReportOOMError();
 @@ -389,6 +391,9 @@ networkStartup(int privileged) {
 goto out_of_memory;
 }

 +if (!(driverState-dnsmasqCaps = dnsmasqCapsNewFromBinary(DNSMASQ))) {
 +/* attempt to continue anyway */
 +}
 Maybe worth a VIR_DEBUG if we are checking for this case.

Well, I could just remove the if() here. I originally *did* have a
VIR_WARN in this case, but removed it because dnsmasqCapsNewFromBinary()
(actually dnsmasqCapsRefresh) logs either an INFO on success (reporting
the version of dnsmasq found and whether or not bind-dynamic is
present), or an ERROR on failure - for example if dnsmasq isn't found,
it will log this:

error : dnsmasqCapsRefresh:703 : Cannot check dnsmasq binary
/sbin/dnsmasq: No such file or directory

libvirtd still starts up with no problem (as long as it doesn't need to
start any networks, in which case it fails those, but still starts).

Looking at this showed me that there is a problem with proper version
detection if dnsmasq doesn't exist when libvirtd is started, but is
installed sometime later, though (it will work, but assumes no
capabilities beyond basic existence). I'm going to make a v2 with that
fixed.



 if (virNetworkLoadAllConfigs(driverState-networks,
  driverState-networkConfigDir,
 @@ -514,6 +519,8 @@ networkShutdown(void) {
 if (driverState-iptables)
 iptablesContextFree(driverState-iptables);

 +virObjectUnref(driverState-dnsmasqCaps);
 +
 networkDriverUnlock(driverState);
 virMutexDestroy(driverState-lock);

 @@ -616,7 +623,8 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
 

Re: [libvirt] RFC: adding configuration for standard dhcp options to network

[Laine Stump]

On 11/24/2012 04:41 PM, Laine Stump wrote:
 There have been multiple requests over the last couple years to make the
 DHCP server of libvirt's virutal networks more configurable (for
 example, see the following BZ:
 https://bugzilla.redhat.com/show_bug.cgi?id=824573 ).

 You can see a full list of the options supported by dnsmasq with this
 command:

dnsmasq --help dhcp

 The biggest question here is in exactly how to represent the options in
 the network XML. I can see a few different options, among them:

 1) make each option its own element within dhcp, e.g.:

  dnsServer family='ipv4' address='x.x.x.x'/
  dnsServer family='ipv6' address=':::1'/
  ntpServer address='y.y.y.y'/
  smtpServer address='z.z.z.z'/

 2) A variation of (1) where each option could appear only once, but with
 two attributes, one for ipv6 address and one for ipv4:

  dnsServer address4='x.x.x.x'address6=':::1'/
  ntpServer address4='y.y.y.y'/

 3) have a repeatable option element (again, a subelement of dhcp
 that can contain any number of the options as attributes:

  option family='ipv4' dnsServer='x.x.x.x' ntpServer='y.y.y.y'
 smtpServer='z.z.z.z'/
  option family='ipv6' dnsServer='::1'/

   (in this case, we may or may not support repeating option to add
 more options for the same family).

 4) yet another variation, where each option is a subelement of option
 family='':

  option family='ipv4'
dnsServerx.x.x.x/dnsServer
ntpServery.y.y.y/ntpServer
smtpServerz.z.z.z/smtpServer
  /option
  option family='ipv6'
dnsServer:::1/dnsServer
  /option

5) With so many options, it may save lots of repetitive code in the
parser/formatter to make a generic option element:

   option name='dns-server' value='x.x.x.x'/
   option name='ntp-server' value='y.y.y.y'/
   option name='smtp-server' value='z.z.z.z'/
   option family='ipv6' name='dns-server' value='::1'/
   option number='201' value='/netboot/boot-dir/' force='yes'/

The final is an example of something I just noticed in the BZ I
mentioned above - it is both using an option number and forcing the
option to be sent back to the client even if the client doesn't request
it. This is apparently needed for some clients.

There is a precedent for the name/value type of object in nwfilter
parameters. The advantages in this case are

  a) the code for the parser couple possibly be much smaller, with just
a single
 enum + VIR_ENUM_(DECL|IMPL)

  b) it would be easier to support the options by number (which are all
defined
 somewhere in RFCs, but not all have mnemonic equivalents in dnsmasq)

On the other hand, if we allowed any possible option by specifying a
number (and even if not, depending on how the parser was implemented),
it would be easier to create a situation where bad stuff could be
injected into the dnsmasq commandline (we should have some type of
intelligent parsing of the values - known options should have a type
associated with them, and unknown should at least be checked for
dangerous characters).

Any opinions from those with more XML experience?




 I'm not sure which of these would be more straightforward for a user.
 One thought I've had is that dnsmasq at least allows specifying
 different groups of guest addresses (with a tag) and applying
 different options to addresses with different tags. I don't know if
 we'll ever use that capability, but if we do it *seems* like any of
 these methods could be adapted to specifying multiple groups.

 Does anyone have a preference? (I have a narrow preference for (1),
 simply because it was the first to come to my mind, but could easily be
 swayed to one of the others, or something else, given a logical reason).

 --
 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] Proposal: no dnsmasq (no dhcp and no dns) and no radvd option

[Gene Czarcinski]

On 11/26/2012 11:19 AM, Gene Czarcinski wrote:

On 11/26/2012 10:40 AM, Gene Czarcinski wrote:
I understand that you can define multiple IPv4 and multiple IPv6 
gateway addresses on a network interface but only one IPv4 DHCP and 
one IPv6 DHCP.  I can see the need for both IPv4 and IPv6 protocols 
on a single network fabric but I am not sure how many real network 
fabrics have multiple subnetworks on them.  Yes, it could be done 
but I am not certain why you would do that (and I am also sure that 
someone has a very valid reason for doing that). 

Oops!  There may be a problem here with radvd!.

I have difficulty in understanding why one would define multiple IPv6 
(or even IPv4) subnetworks on a single interface.  Well, I guess the 
radvd authors did also: the AdvManagedFlag on/off applies to the 
entire interface and no a specific network.


I am verifying this but there is a chance that dsnmasq could support 
both for different subnetworks.


I guess that dnsmasq could be used to support one and radvd used to 
support the other but ???


I believe this may need more discussion from others.  I would like to 
have someone other than the two of us chime in on this.
The answer is not good.  Both radvd and dnsmasq are the same and you 
must choose state-full (DHCPv6) or state-less (SLAAC):


As Simon Kelley says:

 OK, you prompted me to look at the code, which makes radvd's behavior 
more understandable. The Managed flag is in the header of the 
route-advertisement packet so it has, logically, to apply at all the 
prefixes contained therein. The dnsmasq implementation sets the managed 
flag if any of the prefixes has DHCPv6 available, but clients will take 
is applying to them all.


So, for IPv6 on a virtual network you either have one IPv6 subnetwork 
with state-full DHCPv6 or you can have multiple IPv6 subnetworks with 
SLAAC addressing.


Options:

1. Ignore the true situation and keep going.  I believe some users might 
not like this and I certainly do not like this.


2. Start a separate radvd (or dnsmasq) to support state-full DHCPv6 and 
another radvd to support additional SLAAC subnetworks. [Personally, I do 
not like this solution.]  /// The problem is that this solution may not 
work.  /// I just checked and now I remember ... it will not work.  Only 
one RA server per network fabric (think virtual network interface) since 
ff02:: addresses are being used.


3. If an IPv6 DHCP range is specified, then any additional IPv6 
subnetworks are a configuration error.  I believe that this is the only 
reasonable thing to do.  So, if you want to define two IPv6 subnets, do 
it on two different interfaces.  I believe there is not much choice in 
this ... it is just the way IPv6 was defined and works.


[Aside:  I sure would like to know of a real-world need for multiple 
IPv4 or multiple IPv6 subnetworks on a single network fabric.  The 
only possible thing I could think of is the need for a data network and 
a separate control network.  But, from a security perspective, you 
really need to use either networks with encryption separation or real 
hardware separation.]


Gene

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

Re: [libvirt] Proposal: no dnsmasq (no dhcp and no dns) and no radvd option

[Gene Czarcinski]

On 11/25/2012 05:08 PM, Laine Stump wrote:

Speaking of logging, if you do not like the lease renewal messages,
then you will have a bigger problem with the RTR-ADVERT messages
that dnsmasq issues every 3//4/5 seconds for every started interface.

Yes, that's definitely a no-go. I'm pretty sure nobody would stand for
that. If that's how dnsmasq operates, we can't switch to using dnsmasq
until that's remedied.

One-line replacement fix should fix the problem: change LOG_INFO to 
LOG_DEBUG and I am confident that it should be in 2.64.


You mentioned that users were griping about other messages.  I do 
believe that there are too many LOG_INFO messages and perhaps some of 
them need to have more to differentiate them and, even more likely, do 
issue many of those messages unless log-dhcp is specified. [otherwise, 
finding the real information in vast quantities of data becomes very 
challenging]


But, it was also pointed out that too many messages means that you have 
not tuned (filtered) you syslog sufficiently.  That is a very real 
point and could have been used for the RTR-ADVERT messages too except 
they really belongs in LOG_DEBUG.


Gene

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

Re: [libvirt] libvirt [PATCHv2 1/2] Refactor ESX storage driver to implement facade pattern

[Matthias Bolte]

2012/11/10 Ata E Husain Bohra ata.hus...@hotmail.com:
 The patch refactors the current ESX storage driver due to following reasons:

 1. Given most of the public APIs exposed by the storage driver in Libvirt
 remains same, ESX storage driver should not implement logic specific
 for only one supported format (current implementation only supports VMFS).
 2. Decoupling interface from specific storage implementation gives us an
 extensible design to hook implementation for other supported storage
 formats.

 This patch refactors the current driver to implement it as a facade pattern 
 i.e.
 the driver exposes all the public libvirt APIs, but uses backend drivers to 
 get
 the required task done. The backend drivers provide implementation specific to
 the type of storage device.

 File changes:
 --
 esx_storage_driver.c  esx_storage_driver.c (base storage driver)
  |
  |--- esx_storage_backend_vmfs.c (VMFS backend)

 One of the task base storage driver need to do is to decide which backend
 driver needs to be invoked for a given request. This approach extends
 virStoragePool and virStorageVol to store extra parameters:
 1. privateData: stores pointer to respective backend storage driver.
 2. privateDataFreeFunc: stores cleanup function pointer.

 virGetStoragePool and virGetStorageVol are modfiied to accept these extra
 parameters as user params. virStoragePoolDispose and virStorageVolDispose
 checks for cleanup operation if available.
 ---
  daemon/remote.c|6 +-
  src/Makefile.am|1 +
  src/conf/storage_conf.c|3 +-
  src/datatypes.c|   25 +-
  src/datatypes.h|   24 +-
  src/esx/esx_driver.c   |6 +-
  src/esx/esx_storage_backend_vmfs.c | 1491 
 
  src/esx/esx_storage_backend_vmfs.h |   30 +
  src/esx/esx_storage_driver.c   | 1319 +--
  src/esx/esx_vi.c   |5 +-
  src/esx/esx_vi.h   |3 +-
  src/parallels/parallels_storage.c  |   24 +-
  src/remote/remote_driver.c |6 +-
  src/storage/storage_driver.c   |   28 +-
  src/test/test_driver.c |   30 +-
  src/vbox/vbox_tmpl.c   |   14 +-
  16 files changed, 1843 insertions(+), 1172 deletions(-)
  create mode 100644 src/esx/esx_storage_backend_vmfs.c
  create mode 100644 src/esx/esx_storage_backend_vmfs.h

 @@ -444,6 +451,10 @@ virStoragePoolDispose(void *obj)

  VIR_FREE(pool-name);
  virObjectUnref(pool-conn);
 +
 +if (pool-privateDataFreeFunc) {
 +pool-privateDataFreeFunc(pool-privateData);
 +}

This should be done before VIR_FREE(pool-name), because the private
data might store pointer to the owning storage pool and the given free
function might access the storage pool. Calling the free function
before starting to dispose the storage object presents the free
function via a valid storage pool object.

 @@ -520,6 +537,10 @@ virStorageVolDispose(void *obj)
  VIR_FREE(vol-name);
  VIR_FREE(vol-pool);
  virObjectUnref(vol-conn);
 +
 +if (vol-privateDataFreeFunc) {
 +vol-privateDataFreeFunc(vol-privateData);
 +}

The same comment as for virStoragePoolDispose applies here too.

Anyway, I split the changes to virStoragePool/Vol into a separate
patch, applied the mentioned changes, and pushed it.

 diff --git a/src/esx/esx_storage_backend_vmfs.c 
 b/src/esx/esx_storage_backend_vmfs.c
 new file mode 100644
 index 000..d934e57
 --- /dev/null
 +++ b/src/esx/esx_storage_backend_vmfs.c
 @@ -0,0 +1,1491 @@
 +
 +/*
 + * esx_storage_backend_vmfs.c: ESX backend storage driver for
 + * managing VMFS datastores
 + *
 + * Copyright (C) 2012 Ata E Husain Bohra ata.hus...@hotmail.com

Better than before, but as this file contains mostly code from
esx_storage_driver.c with your modifications, the original copyrights
from esx_storage_driver.c still applies here and have to be mentioned.

 +virStorageDriver esxStorageBackendVMFSDrv = {
 +.name = ESX VMFS backend,
 +.open = NULL, /* 1.0.0 */
 +.close = NULL, /* 1.0.0 */
 +.numOfPools = esxStorageBackendVMFSNumberOfStoragePools, /* 1.0.0 */
 +.listPools = esxStorageBackendVMFSListStoragePools, /* 1.0.0 */
 +.poolLookupByName = esxStorageBackendVMFSPoolLookupByName, /* 1.0.0 */
 +.poolLookupByUUID = esxStorageBackendVMFSPoolLookupByUUID, /* 1.0.0 */
 +.poolRefresh = esxStorageBackendVMFSPoolRefresh, /* 1.0.0 */
 +.poolGetInfo = esxStorageBackendVMFSPoolGetInfo, /* 1.0.0 */
 +.poolGetXMLDesc = esxStorageBackendVMFSPoolGetXMLDesc, /* 1.0.0 */
 +.poolNumOfVolumes = esxStorageBackendVMFSPoolNumberOfStorageVolumes, /* 
 1.0.0 */
 +.poolListVolumes = esxStorageBackendVMFSPoolListStorageVolumes, /* 1.0.0 
 */
 +.volLookupByName = esxStorageBackendVMFSVolumeLookupByName, /* 1.0.0 */
 +

[libvirt] [PATCH] build: avoid C99 for loop

[Eric Blake]

Although we require various C99 features, we don't yet require a
complete C99 compiler.  On RHEL 5, compilation complained:

qemu/qemu_command.c: In function 'qemuBuildGraphicsCommandLine':
qemu/qemu_command.c:4688: error: 'for' loop initial declaration used outside 
C99 mode

* src/qemu/qemu_command.c (qemuBuildGraphicsCommandLine): Declare
variable sooner.
* src/qemu/qemu_process.c (qemuProcessInitPasswords): Likewise.
---

Pushing under the build-breaker rule

 src/qemu/qemu_command.c |4 +++-
 src/qemu/qemu_process.c |5 ++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9c9a0ed..03716d4 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4429,6 +4429,8 @@ qemuBuildGraphicsCommandLine(struct qemud_driver *driver,
  qemuCapsPtr caps,
  virDomainGraphicsDefPtr graphics)
 {
+int i;
+
 if (graphics-type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
 virBuffer opt = VIR_BUFFER_INITIALIZER;

@@ -4685,7 +4687,7 @@ qemuBuildGraphicsCommandLine(struct qemud_driver *driver,
 break;
 }

-for (int i = 0 ; i  VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST ; i++) {
+for (i = 0 ; i  VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST ; i++) {
 int mode = graphics-data.spice.channels[i];
 switch (mode) {
 case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE:
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 3d7a5a0..8743c60 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2078,8 +2078,9 @@ qemuProcessInitPasswords(virConnectPtr conn,
 {
 int ret = 0;
 qemuDomainObjPrivatePtr priv = vm-privateData;
+int i;

-for (int i = 0 ; i  vm-def-ngraphics; ++i) {
+for (i = 0 ; i  vm-def-ngraphics; ++i) {
 virDomainGraphicsDefPtr graphics = vm-def-graphics[i];
 if (graphics-type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
 ret = qemuDomainChangeGraphicsPasswords(driver, vm,
@@ -2098,8 +2099,6 @@ qemuProcessInitPasswords(virConnectPtr conn,
 goto cleanup;

 if (qemuCapsGet(priv-caps, QEMU_CAPS_DEVICE)) {
-int i;
-
 for (i = 0 ; i  vm-def-ndisks ; i++) {
 char *secret;
 size_t secretLen;
-- 
1.7.1

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

Re: [libvirt] libvirt [PATCHv2 2/2] Add iSCSI backend storage driver for ESX.

[Matthias Bolte]

2012/11/10 Ata E Husain Bohra ata.hus...@hotmail.com:
 The patch adds the backend driver to support iSCSI format storage pools
 and volumes for ESX host. The mapping of ESX iSCSI specifics to Libvirt
 is as follows:

 1. ESX static iSCSI target -- Libvirt Storage Pools
 2. ESX iSCSI LUNs  -- Libvirt Storage Volumes.

 The above understanding is based on http://libvirt.org/storage.html.

 The operation supported on iSCSI pools includes:

 1. List storage pools  volumes.
 2. Get xml descriptor operaion on pools  volumes.
 3. Lookup operation on pools  volumes by name, uuid and path (if applicable).

 iSCSI pools does not support operations such as: Create / remove pools
 and volumes.

I don't have time for this one today anymore. I'll definitely come
back to it later this week.

-- 
Matthias Bolte
http://photron.blogspot.com

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

Re: [libvirt] [PATCH v1 2/2][RFC] help to create disk images of non-shared migration

[li guang]

Hi, Eric

any comment on this patch?


在 2012-11-23五的 13:28 +0800，liguang写道：
 try to do non-shared migration without bothering to
 create disk images at target by hand.
 
 consider this situation:
 1. non-shared migration
virsh migrate --copy-storage-all ...
 2. migration fails
 3. create disk images required
qemu-img create ...
 4  migration run smoothly
 so, try do remove step 2, 3, 4
 
 this kind of usage had been discussed before,
 http://www.redhat.com/archives/libvir-list/2011-December/msg00451.html
 
 this patch depends on my support offline migration patch:
 https://www.redhat.com/archives/libvir-list/2012-November/msg00512.html
 
 Signed-off-by: liguang lig.f...@cn.fujitsu.com
 ---
  src/qemu/qemu_conf.h  |2 +
  src/qemu/qemu_driver.c|1 +
  src/qemu/qemu_migration.c |  275 
 -
  3 files changed, 277 insertions(+), 1 deletions(-)
 
 diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
 index 47f349c..ad789e9 100644
 --- a/src/qemu/qemu_conf.h
 +++ b/src/qemu/qemu_conf.h
 @@ -102,6 +102,8 @@ struct qemud_driver {
  char *hugetlbfs_mount;
  char *hugepage_path;
  
 +void *private;
 +
  unsigned int macFilter : 1;
  ebtablesContext *ebtables;
  
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index b23056b..db11629 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -1133,6 +1133,7 @@ static virDrvOpenStatus qemudOpen(virConnectPtr conn,
  }
  }
  }
 +qemu_driver-private = conn;
  conn-privateData = qemu_driver;
  
  return VIR_DRV_OPEN_SUCCESS;
 diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
 index 53171df..8b4e563 100644
 --- a/src/qemu/qemu_migration.c
 +++ b/src/qemu/qemu_migration.c
 @@ -50,6 +50,7 @@
  #include storage_file.h
  #include viruri.h
  #include hooks.h
 +#include dirname.h
  
 
  #define VIR_FROM_THIS VIR_FROM_QEMU
 @@ -72,6 +73,7 @@ enum qemuMigrationCookieFlags {
  QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE,
  QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT,
  QEMU_MIGRATION_COOKIE_FLAG_NETWORK,
 +QEMU_MIGRATION_COOKIE_FLAG_COPYSTORAGE,
  
  QEMU_MIGRATION_COOKIE_FLAG_LAST
  };
 @@ -79,13 +81,14 @@ enum qemuMigrationCookieFlags {
  VIR_ENUM_DECL(qemuMigrationCookieFlag);
  VIR_ENUM_IMPL(qemuMigrationCookieFlag,
QEMU_MIGRATION_COOKIE_FLAG_LAST,
 -  graphics, lockstate, persistent, network);
 +  graphics, lockstate, persistent, network, storage);
  
  enum qemuMigrationCookieFeatures {
  QEMU_MIGRATION_COOKIE_GRAPHICS  = (1  
 QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS),
  QEMU_MIGRATION_COOKIE_LOCKSTATE = (1  
 QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE),
  QEMU_MIGRATION_COOKIE_PERSISTENT = (1  
 QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT),
  QEMU_MIGRATION_COOKIE_NETWORK = (1  
 QEMU_MIGRATION_COOKIE_FLAG_NETWORK),
 +QEMU_MIGRATION_COOKIE_COPYSTORAGE = (1  
 QEMU_MIGRATION_COOKIE_FLAG_COPYSTORAGE),
  };
  
  typedef struct _qemuMigrationCookieGraphics qemuMigrationCookieGraphics;
 @@ -119,6 +122,19 @@ struct _qemuMigrationCookieNetwork {
  qemuMigrationCookieNetDataPtr net;
  };
  
 +typedef struct _qemuMigrationCookieStorageData 
 qemuMigrationCookieStorageData;
 +typedef qemuMigrationCookieStorageData *qemuMigrationCookieStorageDataPtr;
 +struct _qemuMigrationCookieStorageData {
 +char *dsize;
 +};
 +
 +typedef struct _qemuMigrationCookieStorage qemuMigrationCookieStorage;
 +typedef qemuMigrationCookieStorage *qemuMigrationCookieStoragePtr;
 +struct _qemuMigrationCookieStorage {
 +int ndisks;
 +qemuMigrationCookieStorageDataPtr disk;
 +};
 +
  typedef struct _qemuMigrationCookie qemuMigrationCookie;
  typedef qemuMigrationCookie *qemuMigrationCookiePtr;
  struct _qemuMigrationCookie {
 @@ -147,6 +163,9 @@ struct _qemuMigrationCookie {
  
  /* If (flags  QEMU_MIGRATION_COOKIE_NETWORK) */
  qemuMigrationCookieNetworkPtr network;
 +
 +/* If (flags  QEMU_MIGRATION_COOKIE_COPYSTORAGE) */
 +qemuMigrationCookieStoragePtr storage;
  };
  
  static void qemuMigrationCookieGraphicsFree(qemuMigrationCookieGraphicsPtr 
 grap)
 @@ -175,6 +194,21 @@ 
 qemuMigrationCookieNetworkFree(qemuMigrationCookieNetworkPtr network)
  VIR_FREE(network);
  }
  
 +static void
 +qemuMigrationCookieStorageFree(qemuMigrationCookieStoragePtr storage)
 +{
 +int i;
 +
 +if (!storage)
 +return;
 +
 +if (storage-disk) {
 +for (i = 0; i  storage-ndisks; i++)
 +VIR_FREE(storage-disk[i].dsize);
 +}
 +VIR_FREE(storage-disk);
 +VIR_FREE(storage);
 +}
  
  static void qemuMigrationCookieFree(qemuMigrationCookiePtr mig)
  {
 @@ -187,6 +221,9 @@ static void 
 qemuMigrationCookieFree(qemuMigrationCookiePtr mig)
  if (mig-flags  QEMU_MIGRATION_COOKIE_NETWORK)
  qemuMigrationCookieNetworkFree(mig-network);
  
 +if (mig-flags  QEMU_MIGRATION_COOKIE_COPYSTORAGE)
 +

Re: [libvirt] [PATCH v15] support offline migration

[li guang]

Hi, Jirka

waiting for your comment ...


在 2012-11-21三的 16:28 +0800，liguang写道：
 original migration did not aware of offline case,
 so, try to support offline migration quietly
 (did not disturb original migration) by pass
 VIR_MIGRATE_OFFLINE flag to migration APIs if only
 the domain is really inactive, and
 migration process will not puzzled by domain
 offline and exit unexpectedly.
 these changes did not take care of disk images the
 domain required, for them could be transferred by
 other APIs as suggested, then VIR_MIGRATE_OFFLINE
 must not combined with VIR_MIGRATE_NON_SHARED_*.
 and you must do a persistent migration at same time,
 do virsh migrate --offline --persistent 
 
 Signed-off-by: liguang lig.f...@cn.fujitsu.com
 ---
  include/libvirt/libvirt.h.in |1 +
  src/libvirt.c|4 +
  src/qemu/qemu_driver.c   |   16 +++---
  src/qemu/qemu_migration.c|  140 
 +++---
  src/qemu/qemu_migration.h|9 ++-
  tools/virsh-domain.c |5 ++
  tools/virsh.pod  |5 +-
  7 files changed, 117 insertions(+), 63 deletions(-)
 
 diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
 index 49a361a..ea625b3 100644
 --- a/include/libvirt/libvirt.h.in
 +++ b/include/libvirt/libvirt.h.in
 @@ -1092,6 +1092,7 @@ typedef enum {
 * whole migration process; 
 this will be used automatically
 * when supported */
  VIR_MIGRATE_UNSAFE= (1  9), /* force migration even if it 
 is considered unsafe */
 +VIR_MIGRATE_OFFLINE   = (1  10), /* offline migrate */
  } virDomainMigrateFlags;
  
  /* Domain migration. */
 diff --git a/src/libvirt.c b/src/libvirt.c
 index bdb1dc6..6d749d9 100644
 --- a/src/libvirt.c
 +++ b/src/libvirt.c
 @@ -4827,6 +4827,10 @@ virDomainMigrateVersion3(virDomainPtr domain,
  if (uri_out)
  uri = uri_out; /* Did domainMigratePrepare3 change URI? */
  
 +if (flags  VIR_MIGRATE_OFFLINE) {
 +cancelled = 0;
 +goto finish;
 +}
  /* Perform the migration.  The driver isn't supposed to return
   * until the migration is complete. The src VM should remain
   * running, but in paused state until the destination can
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 595c452..1ba1665 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -9625,7 +9625,7 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn,
  
  ret = qemuMigrationPrepareTunnel(driver, dconn,
   NULL, 0, NULL, NULL, /* No cookies in 
 v2 */
 - st, dname, dom_xml);
 + st, dname, dom_xml, flags);
  
  cleanup:
  qemuDriverUnlock(driver);
 @@ -9685,7 +9685,7 @@ qemudDomainMigratePrepare2(virConnectPtr dconn,
  ret = qemuMigrationPrepareDirect(driver, dconn,
   NULL, 0, NULL, NULL, /* No cookies */
   uri_in, uri_out,
 - dname, dom_xml);
 + dname, dom_xml, flags);
  
  cleanup:
  qemuDriverUnlock(driver);
 @@ -9827,7 +9827,7 @@ qemuDomainMigrateBegin3(virDomainPtr domain,
  asyncJob = QEMU_ASYNC_JOB_NONE;
  }
  
 -if (!virDomainObjIsActive(vm)) {
 +if (!virDomainObjIsActive(vm)  !(flags  VIR_MIGRATE_OFFLINE)) {
  virReportError(VIR_ERR_OPERATION_INVALID,
 %s, _(domain is not running));
  goto endjob;
 @@ -9836,9 +9836,9 @@ qemuDomainMigrateBegin3(virDomainPtr domain,
  /* Check if there is any ejected media.
   * We don't want to require them on the destination.
   */
 -
 -if (qemuDomainCheckEjectableMedia(driver, vm, asyncJob)  0)
 -goto endjob;
 +if (!(flags  VIR_MIGRATE_OFFLINE) 
 +qemuDomainCheckEjectableMedia(driver, vm, asyncJob)  0)
 +goto endjob;
  
  if (!(xml = qemuMigrationBegin(driver, vm, xmlin, dname,
 cookieout, cookieoutlen,
 @@ -9922,7 +9922,7 @@ qemuDomainMigratePrepare3(virConnectPtr dconn,
   cookiein, cookieinlen,
   cookieout, cookieoutlen,
   uri_in, uri_out,
 - dname, dom_xml);
 + dname, dom_xml, flags);
  
  cleanup:
  qemuDriverUnlock(driver);
 @@ -9967,7 +9967,7 @@ qemuDomainMigratePrepareTunnel3(virConnectPtr dconn,
  ret = qemuMigrationPrepareTunnel(driver, dconn,
   cookiein, cookieinlen,
   cookieout, cookieoutlen,
 - st, dname, dom_xml);
 + st, dname, dom_xml, flags);
   

[libvirt] [test-API][PATCH V2] Add managedsave test cases

[hongming]

The patch V2 inclues fixing all spots commented by Gren
The managedsave test cases and test suite cover test include
verifying virsh commands managedsave(include all flags and their
combination)/managedsave-remove and managedSaveRemove/ManagedSave/
hasManagedSaveImage python APIs.
The following new files be created.
new file: cases/managedsave.conf
- Test all test cases
new file: repos/managedsave/__init__.py
new file: repos/managedsave/managedsave.py
- Test mangaedsave command/API and all flags
new file: repos/managedsave/managedsave_remove.py
- Test managedsave-remove command/API
new file: repos/managedsave/managedsave_start.py
- Verfiy managedsave'flags and start from managedsave image
---
 cases/managedsave.conf  |   63 
 repos/managedsave/managedsave.py|  159 +++
 repos/managedsave/managedsave_remove.py |   60 
 repos/managedsave/managedsave_start.py  |  150 +
 4 files changed, 432 insertions(+), 0 deletions(-)
 create mode 100644 cases/managedsave.conf
 create mode 100644 repos/managedsave/__init__.py
 create mode 100644 repos/managedsave/managedsave.py
 create mode 100644 repos/managedsave/managedsave_remove.py
 create mode 100644 repos/managedsave/managedsave_start.py

diff --git a/cases/managedsave.conf b/cases/managedsave.conf
new file mode 100644
index 000..8dcafe2
--- /dev/null
+++ b/cases/managedsave.conf
@@ -0,0 +1,63 @@
+domain:install_linux_cdrom
+guestname
+$defaultname
+guestos
+$defaultos
+guestarch
+$defaultarch
+vcpu
+$defaultvcpu
+memory
+$defaultmem
+hddriver
+$defaulthd
+nicdriver
+$defaultnic
+imageformat
+qcow2
+macaddr
+54:52:00:4a:16:30
+
+#VIR_DOMAIN_SAVE_BYPASS_CACHE = 1
+#VIR_DOMAIN_SAVE_RUNNING = 2
+#VIR_DOMAIN_SAVE_PAUSED = 4
+#No_FLAGS = 0
+managedsave:managedsave
+guestname
+$defaultname
+flags
+1|2
+
+managedsave:managedsave_start
+guestname
+$defaultname
+flags
+noping
+
+managedsave:managedsave
+guestname
+$defaultname
+flags
+1|4
+
+managedsave:managedsave_start
+guestname
+$defaultname
+flags
+noping
+
+managedsave:managedsave
+guestname
+$defaultname
+flags
+0
+
+managedsave:managedsave_remove
+guestname
+$defaultname
+
+managedsave:managedsave_start
+guestname
+$defaultname
+flags
+noping
diff --git a/repos/managedsave/__init__.py b/repos/managedsave/__init__.py
new file mode 100644
index 000..e69de29
diff --git a/repos/managedsave/managedsave.py b/repos/managedsave/managedsave.py
new file mode 100644
index 000..a126cd8
--- /dev/null
+++ b/repos/managedsave/managedsave.py
@@ -0,0 +1,159 @@
+#!/usr/bin/env python
+
+import os
+import math
+
+import libvirt
+from libvirt import libvirtError
+
+from src import sharedmod
+from utils import utils
+
+required_params = ('guestname', 'flags',)
+optional_params = {}
+
+def check_guest_status(*args):
+Check guest current status
+(domobj, logger) = args
+state = domobj.info()[0]
+logger.debug(current guest status: %s % state)
+
+if state == libvirt.VIR_DOMAIN_SHUTOFF or \
+   state == libvirt.VIR_DOMAIN_SHUTDOWN or \
+   state == libvirt.VIR_DOMAIN_BLOCKED:
+return False
+else:
+return True
+
+def check_savefile_create(*args):
+Check guest's managed save file be created
+
+(guestname) = args
+cmds = ls /var/lib/libvirt/qemu/save/%s % guestname + .save -lh
+logger.info(Execute cmd  %s % cmds) 
+(status, output) = utils.exec_cmd(cmds, shell=True)
+if status != 0:
+logger.error(No managed save file)
+return False
+else :
+logger.info(managed save file exists)
+return True
+
+def compare_cachedfile(cachebefore, cacheafter):
+Compare cached value before managed save and its value after 
+managed save 
+
+diff = cacheafter - cachebefore
+logger.info(diff is %s  % diff)
+percent = math.fabs(diff)/cachebefore
+logger.info(diff percent is %s  % percent)
+if percent  0.05:
+return True
+else:
+return False
+
+def get_cachevalue():
+Get the file system cached value 
+
+cmds = head -n4 /proc/meminfo|grep Cached|awk '{print $2}'
+(status, output) = utils.exec_cmd(cmds, shell=True)
+if status != 0:
+logger.error(Fail to run cmd line to get cache)
+return 1
+else:
+logger.debug(output[0])
+cachevalue= int(output[0])
+return cachevalue
+
+def managedsave(params):
+Managed save a running domain
+
+global logger
+logger = params['logger']
+guestname = params['guestname']
+flags = params ['flags']
+#Save given flags to sharedmod.data
+sharedmod.data['flagsave'] = flags
+
+logger.info(The given flags are 

[libvirt] [test-API][PATCH] balloon_memory: add time break before dump xml

[Wayne Sun]

the xml dumped after setMemory is not accurate due to time
delay, so take 10 sec sleep before dump to show the right
xml info

Signed-off-by: Wayne Sun g...@redhat.com
---
 repos/domain/balloon_memory.py |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/repos/domain/balloon_memory.py b/repos/domain/balloon_memory.py
index 7051a0a..5bf023a 100644
--- a/repos/domain/balloon_memory.py
+++ b/repos/domain/balloon_memory.py
@@ -209,6 +209,7 @@ def balloon_memory(params):
 
 logger.debug(dump the xml description of guest virtual machine %s %
   domname)
+time.sleep(10)
 dom_xml = domobj.XMLDesc(0)
 logger.debug(the xml definination is %s % dom_xml)
 
@@ -240,6 +241,7 @@ def balloon_memory(params):
 
 logger.debug(dump the xml description of \
   guest virtual machine %s % domname)
+time.sleep(10)
 dom_xml = domobj.XMLDesc(0)
 logger.debug(the xml definination is %s % dom_xml)
 
-- 
1.7.1

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

Re: [libvirt] [PATCH 1/6] unpriv_sgio: Add docs and rng schema for new XML unpriv_sgio

[Osier Yang]

On 2012年11月26日 19:26, Paolo Bonzini wrote:

Il 26/11/2012 11:38, Daniel P. Berrange ha scritto:

Since rawio and unpriv_sgio are only valid for lun, this

I think that's a ugly attribute name, just use 'sgio'.


That doesn't explain what it is for.


Yeah, I used sgio in the first drafts, but changed
to unpriv_sgio afterwards, for realizing it can't
tell what the tag does internally.

Some possibilities:


- filter-scsi-commands=yes/no

- cdb-filter=yes/no

- privileged-scsi-commands=allow/deny

- cdb=allow-all/filter


As far as I get from your kernel patches, the unpriv_sgio
allows any SG_IO commands for unprivileged users, without
filtering.

So I'm wondering if using the term CBD filter matches
the exact meaning.

Regards,
Osier

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

Re: [libvirt] [PATCH 1/6] unpriv_sgio: Add docs and rng schema for new XML unpriv_sgio

[Osier Yang]

On 2012年11月27日 15:08, Osier Yang wrote:

On 2012年11月26日 19:26, Paolo Bonzini wrote:

Il 26/11/2012 11:38, Daniel P. Berrange ha scritto:

Since rawio and unpriv_sgio are only valid for lun, this

I think that's a ugly attribute name, just use 'sgio'.


That doesn't explain what it is for.


Yeah, I used sgio in the first drafts, but changed
to unpriv_sgio afterwards, for realizing it can't
tell what the tag does internally.

Some possibilities:


- filter-scsi-commands=yes/no

- cdb-filter=yes/no

- privileged-scsi-commands=allow/deny

- cdb=allow-all/filter


As far as I get from your kernel patches, the unpriv_sgio
allows any SG_IO commands for unprivileged users, without
filtering.


Or do you plan to introduce filtering in future? It sounds
good to use cdbFitler if so.


So I'm wondering if using the term CBD filter matches
the exact meaning.

Regards,
Osier

--
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] Is DBus a hard dependency

[Guido Günther]

On Mon, Nov 26, 2012 at 10:33:15AM +, Daniel P. Berrange wrote:
 On Sat, Nov 24, 2012 at 06:34:35PM +0100, Guido Günther wrote:
  Hi,
  currently running libvirtd without DBus fails due to:
  
  error : nwfilterDriverStartup:208 : DBus matches could not be installed. 
  Disabling nwfilter driver
  error : virDBusGetSystemBus:77 : internal error Unable to get DBus system 
  bus connection: Failed to connect to socket 
  /var/run/dbus/system_bus_socket: No such file or directory
  error : virStateInitialize:810 : Initialization of NWFilter state driver 
  failed
  error : daemonRunStateInit:784 : Driver state initialization failed
  
  because we fail driver initialization hard in nwfilter_driver.c:
  
  if (nwfilterDriverInstallDBusMatches(sysbus)  0) {
  VIR_ERROR(_(DBus matches could not be installed. Disabling 
  nwfilter 
driver));
  /*
   * unfortunately this is fatal since virNWFilterTechDriversInit
   * may have caused the ebiptables driver to use the firewall tool
   * but now that the watches don't work, we just disable the nwfilter
   * driver
   */
  goto error;
  }
  
  I wonder if this on prupose or if we can just make this a soft error and
  go on without DBus? At least in the !HAVE_FIREWALLD case it should be
  o.k. to continue. Shouldn't it? See attached patch.
 
 Generally, if DBus has been requested at compile time, it ought to
 be treated as compulsory, otherwise it should be non-fatal.

Thanks. Good to know.

 
 
  From 22571860568bfe8026e60dcede8f332ec6fd002f Mon Sep 17 00:00:00 2001
  Message-Id: 
  22571860568bfe8026e60dcede8f332ec6fd002f.1353774807.git@sigxcpu.org
  From: =?UTF-8?q?Guido=20G=C3=BCnther?= a...@sigxcpu.org
  Date: Sat, 24 Nov 2012 17:32:59 +0100
  Subject: [PATCH] nwfilter: Allow DBus initialization to fail
  To: libvir-list@redhat.com
  
  in case we don't use firewalld. This allows us to run without
  DBus on servers.
  ---
   src/nwfilter/nwfilter_driver.c |2 ++
   1 file changed, 2 insertions(+)
  
  diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
  index 12f47ef..e4f6ec9 100644
  --- a/src/nwfilter/nwfilter_driver.c
  +++ b/src/nwfilter/nwfilter_driver.c
  @@ -204,6 +204,7 @@ nwfilterDriverStartup(int privileged)
* initializing
*/
   if (nwfilterDriverInstallDBusMatches(sysbus)  0) {
  +#if HAVE_FIREWALLD
   VIR_ERROR(_(DBus matches could not be installed. Disabling 
  nwfilter 
 driver));
   /*
  @@ -213,6 +214,7 @@ nwfilterDriverStartup(int privileged)
* driver
*/
   goto error;
  +#endif
   }
 
 IMHO any conditional should be in the nwfilterDriverInstallDBusMatches method
 itself. ie that method should be a no-op hardcoded to return 0, if firewalld
 has been disabled at compile time.

It turns out that this is already the case - I missed it since I only
tested with this patch and --without-firewalld. 

So building --with-firewalld currently needs dbus and firewalld for
libvirtd to actually start.
Thanks,
 -- Guido

 
 Daniel
 -- 
 |: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
 |: http://libvirt.org  -o- http://virt-manager.org :|
 |: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
 |: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|
 

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