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

2014-07-10 Thread Ján Tomko
On 07/08/2014 10:32 AM, Chen Hanxiao wrote:
 In lxc, we could not use setmem command
 with --config options.
 This patch will add support for this.
 
 Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
 ---
 v2: use virDomainSetMemoryFlagsEnsureACL
 remove redundant domain running check
 
  src/lxc/lxc_driver.c | 48 +---
  1 file changed, 37 insertions(+), 11 deletions(-)
 
 diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
 index b47ac5e..93f496b 100644
 --- a/src/lxc/lxc_driver.c
 +++ b/src/lxc/lxc_driver.c
 @@ -711,18 +711,33 @@ static int lxcDomainSetMaxMemory(virDomainPtr dom, 
 unsigned long newmax)
  return ret;
  }
  
 -static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem)
 +static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
 +   unsigned int flags)
  {
  virDomainObjPtr vm;
 +virDomainDefPtr persistentDef = NULL;
 +virCapsPtr caps = NULL;
  int ret = -1;
  virLXCDomainObjPrivatePtr priv;
 +virLXCDriverPtr driver = dom-conn-privateData;
 +virLXCDriverConfigPtr cfg = NULL;
 +
 +virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
 +  VIR_DOMAIN_AFFECT_CONFIG, -1);
  
  if (!(vm = lxcDomObjFromDomain(dom)))
  goto cleanup;
  
  priv = vm-privateData;
  
 -if (virDomainSetMemoryEnsureACL(dom-conn, vm-def)  0)
 +if (virDomainSetMemoryFlagsEnsureACL(dom-conn, vm-def, flags)  0)
 +goto cleanup;
 +
 +if (!(caps = virLXCDriverGetCapabilities(driver, false)))
 +goto cleanup;
 +
 +if (virDomainLiveConfigHelperMethod(caps, driver-xmlopt, vm, flags,
 +persistentDef)  0)
  goto cleanup;
  

  if (newmem  vm-def-mem.max_balloon) {

This check should only be done for AFFECT_LIVE.
For AFFECT_CONFIG it needs to be checked against the max_balloon value from
the persistent definition.

Jan



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

[libvirt] [PREPOST 02/17] src/xenxs:Introduce function

2014-07-10 Thread David Kiarie
From: Kiarie Kahurani davidkiar...@gmail.com

Introduce function xenParseXMMem(virConfPtr conf,);
This function parses the xm memory options

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

diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 1953a85..dc840e5 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -294,6 +294,21 @@ static int xenParseXMGeneral(virConfPtr conf, 
virDomainDefPtr def,
 }
 return 0;
 }
+static int xenParseXMMem(virConfPtr conf, virDomainDefPtr def)
+{
+if (xenXMConfigGetULongLong(conf, memory, def-mem.cur_balloon,
+MIN_XEN_GUEST_SIZE * 2)  0)
+return -1;
+
+if (xenXMConfigGetULongLong(conf, maxmem, def-mem.max_balloon,
+def-mem.cur_balloon)  0)
+return -1;
+
+def-mem.cur_balloon *= 1024;
+def-mem.max_balloon *= 1024;
+
+return 0;
+}
 virDomainDefPtr
 xenParseXM(virConfPtr conf, int xendConfigVersion,
virCapsPtr caps)
@@ -372,18 +387,9 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
 goto cleanup;
 }
 }
-
-if (xenXMConfigGetULongLong(conf, memory, def-mem.cur_balloon,
-MIN_XEN_GUEST_SIZE * 2)  0)
-goto cleanup;
-
-if (xenXMConfigGetULongLong(conf, maxmem, def-mem.max_balloon,
-def-mem.cur_balloon)  0)
+if (xenParseXMMem(conf, def)  0)
 goto cleanup;
 
-def-mem.cur_balloon *= 1024;
-def-mem.max_balloon *= 1024;
-
 if (xenXMConfigGetULong(conf, vcpus, count, 1)  0 ||
 MAX_VIRT_CPUS  count)
 goto cleanup;
-- 
1.8.4.5

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


[libvirt] [PREPOST 03/17] src/xenxm:Refactor network config parsing code

2014-07-10 Thread David Kiarie
From: Kiarie Kahurani davidkiar...@gmail.com

I introduced the function
  xenFormatXMVif(virConfPtr conf, ..);
which parses network configuration

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

diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index dc840e5..26ebd5a 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -309,6 +309,159 @@ static int xenParseXMMem(virConfPtr conf, virDomainDefPtr 
def)
 
 return 0;
 }
+static int xenParseXMVif(virConfPtr conf, virDomainDefPtr def)
+{
+char *script = NULL;
+virDomainNetDefPtr net = NULL;
+virConfValuePtr list = virConfGetValue(conf, vif);
+if (list  list-type == VIR_CONF_LIST) {
+list = list-list;
+while (list) {
+char model[10];
+char type[10];
+char ip[16];
+char mac[18];
+char bridge[50];
+char vifname[50];
+char *key;
+
+bridge[0] = '\0';
+mac[0] = '\0';
+ip[0] = '\0';
+model[0] = '\0';
+type[0] = '\0';
+vifname[0] = '\0';
+
+if ((list-type != VIR_CONF_STRING) || (list-str == NULL))
+goto skipnic;
+
+key = list-str;
+while (key) {
+char *data;
+char *nextkey = strchr(key, ',');
+
+if (!(data = strchr(key, '=')))
+goto skipnic;
+data++;
+
+if (STRPREFIX(key, mac=)) {
+int len = nextkey ? (nextkey - data) : sizeof(mac) - 1;
+if (virStrncpy(mac, data, len, sizeof(mac)) == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(MAC address %s too big for 
destination),
+   data);
+goto skipnic;
+}
+} else if (STRPREFIX(key, bridge=)) {
+int len = nextkey ? (nextkey - data) : sizeof(bridge) - 1;
+if (virStrncpy(bridge, data, len, sizeof(bridge)) == NULL) 
{
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Bridge %s too big for destination),
+   data);
+goto skipnic;
+}
+} else if (STRPREFIX(key, script=)) {
+int len = nextkey ? (nextkey - data) : strlen(data);
+VIR_FREE(script);
+if (VIR_STRNDUP(script, data, len)  0)
+goto cleanup;
+} else if (STRPREFIX(key, model=)) {
+int len = nextkey ? (nextkey - data) : sizeof(model) - 1;
+if (virStrncpy(model, data, len, sizeof(model)) == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Model %s too big for destination), 
data);
+goto skipnic;
+}
+} else if (STRPREFIX(key, type=)) {
+int len = nextkey ? (nextkey - data) : sizeof(type) - 1;
+if (virStrncpy(type, data, len, sizeof(type)) == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Type %s too big for destination), 
data);
+goto skipnic;
+}
+} else if (STRPREFIX(key, vifname=)) {
+int len = nextkey ? (nextkey - data) : sizeof(vifname) - 1;
+if (virStrncpy(vifname, data, len, sizeof(vifname)) == 
NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Vifname %s too big for destination),
+   data);
+goto skipnic;
+}
+} else if (STRPREFIX(key, ip=)) {
+int len = nextkey ? (nextkey - data) : sizeof(ip) - 1;
+if (virStrncpy(ip, data, len, sizeof(ip)) == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(IP %s too big for destination), 
data);
+goto skipnic;
+}
+}
+
+while (nextkey  (nextkey[0] == ',' ||
+   nextkey[0] == ' ' ||
+   nextkey[0] == '\t'))
+nextkey++;
+key = nextkey;
+}
+
+if (VIR_ALLOC(net)  0)
+goto cleanup;
+
+if (mac[0]) {
+if (virMacAddrParse(mac, net-mac)  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+

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

2014-07-10 Thread David Kiarie
From: Kiarie Kahurani davidkiar...@gmail.com

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

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

diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index e30ab9c..7a5b47c 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -529,6 +529,102 @@ static int xenParseXMEventsActions(virConfPtr conf, 
virDomainDefPtr def)
 return 0;
 
 }
+static int
+xenParseXMPCI(virConfPtr conf, virDomainDefPtr def)
+{
+virConfValuePtr list = virConfGetValue(conf, pci);
+virDomainHostdevDefPtr hostdev = NULL;
+if (list  list-type == VIR_CONF_LIST) {
+list = list-list;
+while (list) {
+char domain[5];
+char bus[3];
+char slot[3];
+char func[2];
+char *key, *nextkey;
+int domainID;
+int busID;
+int slotID;
+int funcID;
+
+domain[0] = bus[0] = slot[0] = func[0] = '\0';
+
+if ((list-type != VIR_CONF_STRING) || (list-str == NULL))
+goto skippci;
+
+/* pci=[':00:1b.0',':00:13.0'] */
+if (!(key = list-str))
+goto skippci;
+if (!(nextkey = strchr(key, ':')))
+goto skippci;
+
+if (virStrncpy(domain, key, (nextkey - key), sizeof(domain)) == 
NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Domain %s too big for destination), key);
+goto skippci;
+}
+
+key = nextkey + 1;
+if (!(nextkey = strchr(key, ':')))
+goto skippci;
+
+if (virStrncpy(bus, key, (nextkey - key), sizeof(bus)) == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Bus %s too big for destination), key);
+goto skippci;
+}
+
+key = nextkey + 1;
+if (!(nextkey = strchr(key, '.')))
+goto skippci;
+
+if (virStrncpy(slot, key, (nextkey - key), sizeof(slot)) == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Slot %s too big for destination), key);
+goto skippci;
+}
+
+key = nextkey + 1;
+if (strlen(key) != 1)
+goto skippci;
+
+if (virStrncpy(func, key, 1, sizeof(func)) == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Function %s too big for destination), key);
+goto skippci;
+}
+
+if (virStrToLong_i(domain, NULL, 16, domainID)  0)
+goto skippci;
+if (virStrToLong_i(bus, NULL, 16, busID)  0)
+goto skippci;
+if (virStrToLong_i(slot, NULL, 16, slotID)  0)
+goto skippci;
+if (virStrToLong_i(func, NULL, 16, funcID)  0)
+goto skippci;
+
+if (!(hostdev = virDomainHostdevDefAlloc()))
+   return -1;
+
+hostdev-managed = false;
+hostdev-source.subsys.type = VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI;
+hostdev-source.subsys.u.pci.addr.domain = domainID;
+hostdev-source.subsys.u.pci.addr.bus = busID;
+hostdev-source.subsys.u.pci.addr.slot = slotID;
+hostdev-source.subsys.u.pci.addr.function = funcID;
+
+if (VIR_APPEND_ELEMENT(def-hostdevs, def-nhostdevs, hostdev)  
0) {
+virDomainHostdevDefFree(hostdev);
+return -1;
+}
+
+skippci:
+list = list-next;
+}
+}
+
+return 0;
+}
 virDomainDefPtr
 xenParseXM(virConfPtr conf, int xendConfigVersion,
virCapsPtr caps)
@@ -541,7 +637,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
 virDomainDiskDefPtr disk = NULL;
 virDomainNetDefPtr net = NULL;
 virDomainGraphicsDefPtr graphics = NULL;
-virDomainHostdevDefPtr hostdev = NULL;
 size_t i;
 unsigned long count;
 char *script = NULL;
@@ -848,98 +943,9 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
goto cleanup;
if (xenParseXMEventsActions(conf, def)  0)
goto cleanup;
-
-list = virConfGetValue(conf, pci);
-if (list  list-type == VIR_CONF_LIST) {
-list = list-list;
-while (list) {
-char domain[5];
-char bus[3];
-char slot[3];
-char func[2];
-char *key, *nextkey;
-int domainID;
-int busID;
-int slotID;
-int funcID;
-
-domain[0] = bus[0] = slot[0] = func[0] = '\0';
-
-if ((list-type != VIR_CONF_STRING) || (list-str == NULL))
-goto 

[libvirt] [PREPOST 10/17] src/xenxs:Refactor xenParseXM( )

2014-07-10 Thread David Kiarie
From: Kiarie Kahurani davidkiar...@gmail.com

A couple of miscellaneous fixed and wrap code common code into
xenParseConfigCommon(...).I will drop some of the functions later
though

signed-off-by: David Kiariedavidkiar...@gmail.com
---
 src/xenxs/xen_xm.c | 134 -
 src/xenxs/xen_xm.h |   3 +-
 2 files changed, 73 insertions(+), 64 deletions(-)

diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 312d890..657dd1c 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -458,7 +458,7 @@ static int xenParseXMVif(virConfPtr conf, virDomainDefPtr 
def)
 }
 
 return 0;
-cleanup:
+ cleanup:
 VIR_FREE(script);
 return -1;
 }
@@ -1026,7 +1026,8 @@ static int xenParseXMOS(virConfPtr conf, virDomainDefPtr 
def)
 
 if (STREQ(def-os.type, hvm)) {
 const char *boot;
-
+if (xenXMConfigCopyStringOpt(conf, device_model, def-emulator)  0)
+return -1;
 if (xenXMConfigCopyStringOpt(conf, device_model, def-emulator)  0)
 return -1;
 
@@ -1122,54 +1123,13 @@ static int xenParseXMMisc(virConfPtr conf, 
virDomainDefPtr def)
 
 return 0;
 }
-virDomainDefPtr
-xenParseXM(virConfPtr conf, int xendConfigVersion,
-   virCapsPtr caps)
+static int xenParseXMCharDev(virConfPtr conf, virDomainDefPtr def)
 {
 const char *str;
-int hvm = 0;
-virConfValuePtr list;
-virDomainDefPtr def = NULL;
-virDomainDiskDefPtr disk = NULL;
-virDomainNetDefPtr net = NULL;
-virDomainGraphicsDefPtr graphics = NULL;
-char *script = NULL;
-char *listenAddr = NULL;
-
-if (VIR_ALLOC(def)  0)
-return NULL;
-
-def-virtType = VIR_DOMAIN_VIRT_XEN;
-def-id = -1;
-if (xenParseXMGeneral(conf, def, caps)  0)
-goto cleanup;
-
-if (xenParseXMOS(conf, def)  0)
-goto cleanup;
+virConfValuePtr value = NULL;
+virDomainChrDefPtr chr = NULL;
 
-hvm = (STREQ(def-os.type, hvm));
-if (xenParseXMMem(conf, def)  0)
-goto cleanup;
-if (xenXMConfigCopyStringOpt(conf, device_model, def-emulator)  0)
-goto cleanup;
-if (xenParseXMVif(conf, def)  0)
-goto cleanup;
-if (xenParseXMTimeOffset(conf, def, xendConfigVersion)  0)
-goto cleanup;
-if (xenParseXMEventsActions(conf, def)  0)
-goto cleanup;
-if (xenParseXMPCI(conf, def)  0)
-goto cleanup;
-if (xenParseXMCPUFeatures(conf, def)  0)
-goto cleanup;
-if (xenParseXMDisk(conf, def, xendConfigVersion)  0)
-goto cleanup;
-if (xenParseXMVfb(conf, def, xendConfigVersion)  0)
-goto cleanup;
-if (xenParseXMMisc(conf, def)  0)
-goto cleanup;
-if (hvm) {
-virDomainChrDefPtr chr = NULL;
+if (STREQ(def-os.type, hvm)) {
 
 if (xenXMConfigGetString(conf, parallel, str, NULL)  0)
 goto cleanup;
@@ -1179,7 +1139,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
 
 if (chr) {
 if (VIR_ALLOC_N(def-parallels, 1)  0) {
-virDomainChrDefFree(chr);
 goto cleanup;
 }
 chr-deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL;
@@ -1190,21 +1149,21 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
 }
 
 /* Try to get the list of values to support multiple serial ports */
-list = virConfGetValue(conf, serial);
-if (list  list-type == VIR_CONF_LIST) {
+value = virConfGetValue(conf, serial);
+if (value  value-type == VIR_CONF_LIST) {
 int portnum = -1;
 
-list = list-list;
-while (list) {
+value = value-list;
+while (value) {
 char *port = NULL;
 
-if ((list-type != VIR_CONF_STRING) || (list-str == NULL))
+if ((value-type != VIR_CONF_STRING) || (value-str == NULL))
 goto cleanup;
 
-port = list-str;
+port = value-str;
 portnum++;
 if (STREQ(port, none)) {
-list = list-next;
+value = value-next;
 continue;
 }
 
@@ -1215,11 +1174,10 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
 chr-target.port = portnum;
 
 if (VIR_APPEND_ELEMENT(def-serials, def-nserials, chr)  0) {
-virDomainChrDefFree(chr);
 goto cleanup;
 }
 
-list = list-next;
+value = value-next;
 }
 } else {
 /* If domain is not using multiple serial ports we parse data old 
way */
@@ -1249,16 +1207,66 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
 def-consoles[0]-target.port = 0;
 def-consoles[0]-targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN;
 }
-VIR_FREE(script);
+
+return 0;
+
+ cleanup:
+virDomainChrDefFree(chr);
+

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

2014-07-10 Thread David Kiarie
From: Kiarie Kahurani davidkiar...@gmail.com

Introduce the function
 xenParseXMDisk(..);

Parses disk config

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

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

[libvirt] [PREPOST 11/17] src/xenxs:Refactor code formating general information

2014-07-10 Thread David Kiarie
From: Kiarie Kahurani davidkiar...@gmail.com

refactor code parsing uuid, memory and name options

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

diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 657dd1c..a907252 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1609,6 +1609,31 @@ xenFormatXMPCI(virConfPtr conf,
either 32, or 64 on a platform where long is big enough.  */
 verify(MAX_VIRT_CPUS = sizeof(1UL) * CHAR_BIT);
 
+static int xenFormatXMGeneral(virConfPtr conf, virDomainDefPtr def)
+{
+char uuid[VIR_UUID_STRING_BUFLEN];
+
+if (xenXMConfigSetString(conf, name, def-name)  0)
+return -1;
+
+virUUIDFormat(def-uuid, uuid);
+if (xenXMConfigSetString(conf, uuid, uuid)  0)
+return -1;
+
+return 0;
+}
+static int xenFormatXMMem(virConfPtr conf, virDomainDefPtr def)
+{
+if (xenXMConfigSetInt(conf, maxmem,
+  VIR_DIV_UP(def-mem.max_balloon, 1024))  0)
+return -1;
+
+if (xenXMConfigSetInt(conf, memory,
+  VIR_DIV_UP(def-mem.cur_balloon, 1024))  0)
+return -1;
+
+return 0;
+}
 virConfPtr xenFormatXM(virConnectPtr conn,
virDomainDefPtr def,
int xendConfigVersion)
@@ -1618,27 +1643,16 @@ virConfPtr xenFormatXM(virConnectPtr conn,
 size_t i;
 char *cpus = NULL;
 const char *lifecycle;
-char uuid[VIR_UUID_STRING_BUFLEN];
 virConfValuePtr diskVal = NULL;
 virConfValuePtr netVal = NULL;
 
 if (!(conf = virConfNew()))
 goto cleanup;
 
-
-if (xenXMConfigSetString(conf, name, def-name)  0)
-goto cleanup;
-
-virUUIDFormat(def-uuid, uuid);
-if (xenXMConfigSetString(conf, uuid, uuid)  0)
+if (xenFormatXMGeneral(conf, def)  0)
 goto cleanup;
 
-if (xenXMConfigSetInt(conf, maxmem,
-  VIR_DIV_UP(def-mem.max_balloon, 1024))  0)
-goto cleanup;
-
-if (xenXMConfigSetInt(conf, memory,
-  VIR_DIV_UP(def-mem.cur_balloon, 1024))  0)
+if (xenFormatXMMem(conf, def)  0)
 goto cleanup;
 
 if (xenXMConfigSetInt(conf, vcpus, def-maxvcpus)  0)
-- 
1.8.4.5

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


[libvirt] [PREPOST 04/17] src/xenxs:Refactor code parsing time and event actions config

2014-07-10 Thread David Kiarie
From: Kiarie Kahurani davidkiar...@gmail.com

Introduce the functions
 xenParseXMTimeOffset(...);
 xenParseXMEventActions(..);
These parse the time config and Event actions configs
respectively

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

diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 26ebd5a..e30ab9c 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -462,6 +462,73 @@ cleanup:
 VIR_FREE(script);
 return -1;
 }
+static int xenParseXMTimeOffset(virConfPtr conf, virDomainDefPtr def,
+int xendConfigVersion)
+{
+int vmlocaltime;
+
+if (xenXMConfigGetBool(conf, localtime, vmlocaltime, 0)  0)
+return -1;
+
+if (STREQ(def-os.type, hvm)) {
+/* only managed HVM domains since 3.1.0 have persistent rtc_timeoffset 
*/
+if (xendConfigVersion  XEND_CONFIG_VERSION_3_1_0) {
+if (vmlocaltime)
+def-clock.offset = VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME;
+else
+def-clock.offset = VIR_DOMAIN_CLOCK_OFFSET_UTC;
+def-clock.data.utc_reset = true;
+} else {
+unsigned long rtc_timeoffset;
+def-clock.offset = VIR_DOMAIN_CLOCK_OFFSET_VARIABLE;
+if (xenXMConfigGetULong(conf, rtc_timeoffset, rtc_timeoffset, 
0)  0)
+return -1;
+def-clock.data.variable.adjustment = (int)rtc_timeoffset;
+def-clock.data.variable.basis = vmlocaltime ?
+VIR_DOMAIN_CLOCK_BASIS_LOCALTIME :
+VIR_DOMAIN_CLOCK_BASIS_UTC;
+}
+} else {
+/* PV domains do not have an emulated RTC and the offset is fixed. */
+def-clock.offset = vmlocaltime ?
+VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME :
+VIR_DOMAIN_CLOCK_OFFSET_UTC;
+def-clock.data.utc_reset = true;
+} /* !hvm */
+
+return 0;
+}
+static int xenParseXMEventsActions(virConfPtr conf, virDomainDefPtr def)
+{
+const char *str;
+
+if (xenXMConfigGetString(conf, on_poweroff, str, destroy)  0)
+return -1;
+if ((def-onPoweroff = virDomainLifecycleTypeFromString(str))  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(unexpected value %s for on_poweroff), str);
+return -1;
+}
+
+if (xenXMConfigGetString(conf, on_reboot, str, restart)  0)
+return -1;
+if ((def-onReboot = virDomainLifecycleTypeFromString(str))  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(unexpected value %s for on_reboot), str);
+return -1;
+}
+
+if (xenXMConfigGetString(conf, on_crash, str, restart)  0)
+return -1;
+if ((def-onCrash = virDomainLifecycleCrashTypeFromString(str))  0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(unexpected value %s for on_crash), str);
+return -1;
+}
+
+return 0;
+
+}
 virDomainDefPtr
 xenParseXM(virConfPtr conf, int xendConfigVersion,
virCapsPtr caps)
@@ -476,7 +543,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
 virDomainGraphicsDefPtr graphics = NULL;
 virDomainHostdevDefPtr hostdev = NULL;
 size_t i;
-int vmlocaltime = 0;
 unsigned long count;
 char *script = NULL;
 char *listenAddr = NULL;
@@ -556,32 +622,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
 if (str  (virBitmapParse(str, 0, def-cpumask, 4096)  0))
 goto cleanup;
 
-if (xenXMConfigGetString(conf, on_poweroff, str, destroy)  0)
-goto cleanup;
-if ((def-onPoweroff = virDomainLifecycleTypeFromString(str))  0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(unexpected value %s for on_poweroff), str);
-goto cleanup;
-}
-
-if (xenXMConfigGetString(conf, on_reboot, str, restart)  0)
-goto cleanup;
-if ((def-onReboot = virDomainLifecycleTypeFromString(str))  0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(unexpected value %s for on_reboot), str);
-goto cleanup;
-}
-
-if (xenXMConfigGetString(conf, on_crash, str, restart)  0)
-goto cleanup;
-if ((def-onCrash = virDomainLifecycleCrashTypeFromString(str))  0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(unexpected value %s for on_crash), str);
-goto cleanup;
-}
-
-
-
 if (hvm) {
 if (xenXMConfigGetBool(conf, pae, val, 0)  0)
 goto cleanup;
@@ -621,35 +661,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
 def-clock.timers[0] = timer;
 }
 }
-if (xenXMConfigGetBool(conf, localtime, vmlocaltime, 0)  0)
-goto cleanup;
-
-if (hvm) {
-/* only managed HVM domains since 3.1.0 have persistent rtc_timeoffset 
*/
-if 

[libvirt] [PREPOST 13/17] src/xenxs:Refactor code formating event actions config

2014-07-10 Thread David Kiarie
From: Kiarie Kahurani davidkiar...@gmail.com

Introduce function
xenFormatXMEventActions(.)
which formats actions following certain events

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

diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index c91fa73..367a8cd 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1710,6 +1710,38 @@ static int xenFormatXMTimeOffset(virConfPtr conf, 
virDomainDefPtr def,
 
 return 0;
 }
+static int xenFormatXMEventActions(virConfPtr conf, virDomainDefPtr def)
+{
+const char *lifecycle = NULL;
+
+if (!(lifecycle = virDomainLifecycleTypeToString(def-onPoweroff))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(unexpected lifecycle action %d), def-onPoweroff);
+return -1;
+}
+if (xenXMConfigSetString(conf, on_poweroff, lifecycle)  0)
+return -1;
+
+
+if (!(lifecycle = virDomainLifecycleTypeToString(def-onReboot))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(unexpected lifecycle action %d), def-onReboot);
+return -1;
+}
+if (xenXMConfigSetString(conf, on_reboot, lifecycle)  0)
+return -1;
+
+
+if (!(lifecycle = virDomainLifecycleCrashTypeToString(def-onCrash))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(unexpected lifecycle action %d), def-onCrash);
+return -1;
+}
+if (xenXMConfigSetString(conf, on_crash, lifecycle)  0)
+return -1;
+
+return 0;
+}
 virConfPtr xenFormatXM(virConnectPtr conn,
virDomainDefPtr def,
int xendConfigVersion)
@@ -1718,7 +1750,6 @@ virConfPtr xenFormatXM(virConnectPtr conn,
 int hvm = 0;
 size_t i;
 char *cpus = NULL;
-const char *lifecycle;
 virConfValuePtr diskVal = NULL;
 virConfValuePtr netVal = NULL;
 
@@ -1857,33 +1888,8 @@ virConfPtr xenFormatXM(virConnectPtr conn,
 if (xenFormatXMTimeOffset(conf, def, xendConfigVersion)  0)
 goto cleanup;
 
-if (!(lifecycle = virDomainLifecycleTypeToString(def-onPoweroff))) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(unexpected lifecycle action %d), def-onPoweroff);
-goto cleanup;
-}
-if (xenXMConfigSetString(conf, on_poweroff, lifecycle)  0)
-goto cleanup;
-
-
-if (!(lifecycle = virDomainLifecycleTypeToString(def-onReboot))) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(unexpected lifecycle action %d), def-onReboot);
-goto cleanup;
-}
-if (xenXMConfigSetString(conf, on_reboot, lifecycle)  0)
-goto cleanup;
-
-
-if (!(lifecycle = virDomainLifecycleCrashTypeToString(def-onCrash))) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _(unexpected lifecycle action %d), def-onCrash);
+if (xenFormatXMEventActions(conf, def)  0)
 goto cleanup;
-}
-if (xenXMConfigSetString(conf, on_crash, lifecycle)  0)
-goto cleanup;
-
-
 
 if (hvm) {
 if (def-emulator 
-- 
1.8.4.5

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


[libvirt] [PREPOST 16/17] src/xenxs:Refactor Vfb config parsing code

2014-07-10 Thread David Kiarie
From: Kiarie Kahurani davidkiar...@gmail.com

Introduce function
 xenFormatXMVfb(...);
which formats Vfb config

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

diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 8dd2823..613730f 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1851,6 +1851,111 @@ static int xenFormatXMDomainNet(virConfPtr conf, 
virConnectPtr conn,
 VIR_FREE(netVal);
 return -1;
 }
+static int xenFormatXMVfb(virConfPtr conf, virDomainDefPtr def,
+  int xendConfigVersion)
+{
+int hvm = STREQ(def-os.type, hvm);
+if (def-ngraphics == 1) {
+if (hvm || (xendConfigVersion  XEND_CONFIG_MIN_VERS_PVFB_NEWCONF)) {
+if (def-graphics[0]-type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) {
+if (xenXMConfigSetInt(conf, sdl, 1)  0)
+return -1;
+if (xenXMConfigSetInt(conf, vnc, 0)  0)
+return -1;
+if (def-graphics[0]-data.sdl.display 
+xenXMConfigSetString(conf, display,
+ def-graphics[0]-data.sdl.display)  0)
+return -1;
+if (def-graphics[0]-data.sdl.xauth 
+xenXMConfigSetString(conf, xauthority,
+ def-graphics[0]-data.sdl.xauth)  0)
+return -1;
+} else {
+const char *listenAddr;
+
+if (xenXMConfigSetInt(conf, sdl, 0)  0)
+return -1;
+if (xenXMConfigSetInt(conf, vnc, 1)  0)
+return -1;
+if (xenXMConfigSetInt(conf, vncunused,
+  def-graphics[0]-data.vnc.autoport ? 1 : 0)  0)
+return -1;
+if (!def-graphics[0]-data.vnc.autoport 
+xenXMConfigSetInt(conf, vncdisplay,
+  def-graphics[0]-data.vnc.port - 5900)  0)
+return -1;
+listenAddr = 
virDomainGraphicsListenGetAddress(def-graphics[0], 0);
+if (listenAddr 
+xenXMConfigSetString(conf, vnclisten, listenAddr)  0)
+return -1;
+if (def-graphics[0]-data.vnc.auth.passwd 
+xenXMConfigSetString(conf, vncpasswd,
+
def-graphics[0]-data.vnc.auth.passwd)  0)
+return -1;
+if (def-graphics[0]-data.vnc.keymap 
+xenXMConfigSetString(conf, keymap,
+def-graphics[0]-data.vnc.keymap)  0)
+return -1;
+}
+} else {
+virConfValuePtr vfb, disp;
+char *vfbstr = NULL;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+if (def-graphics[0]-type == VIR_DOMAIN_GRAPHICS_TYPE_SDL) {
+virBufferAddLit(buf, type=sdl);
+if (def-graphics[0]-data.sdl.display)
+virBufferAsprintf(buf, ,display=%s,
+  def-graphics[0]-data.sdl.display);
+if (def-graphics[0]-data.sdl.xauth)
+virBufferAsprintf(buf, ,xauthority=%s,
+  def-graphics[0]-data.sdl.xauth);
+} else {
+const char *listenAddr
+= virDomainGraphicsListenGetAddress(def-graphics[0], 0);
+
+virBufferAddLit(buf, type=vnc);
+virBufferAsprintf(buf, ,vncunused=%d,
+  def-graphics[0]-data.vnc.autoport ? 1 : 0);
+if (!def-graphics[0]-data.vnc.autoport)
+virBufferAsprintf(buf, ,vncdisplay=%d,
+  def-graphics[0]-data.vnc.port - 5900);
+if (listenAddr)
+virBufferAsprintf(buf, ,vnclisten=%s, listenAddr);
+if (def-graphics[0]-data.vnc.auth.passwd)
+virBufferAsprintf(buf, ,vncpasswd=%s,
+  def-graphics[0]-data.vnc.auth.passwd);
+if (def-graphics[0]-data.vnc.keymap)
+virBufferAsprintf(buf, ,keymap=%s,
+  def-graphics[0]-data.vnc.keymap);
+}
+if (virBufferCheckError(buf)  0)
+return -1;
+
+vfbstr = virBufferContentAndReset(buf);
+
+if (VIR_ALLOC(vfb)  0) {
+VIR_FREE(vfbstr);
+return -1;
+}
+
+if (VIR_ALLOC(disp)  0) {
+VIR_FREE(vfb);
+VIR_FREE(vfbstr);
+return -1;
+}
+
+vfb-type = VIR_CONF_LIST;
+vfb-list = disp;
+  

[libvirt] [PREPOST 08/17] src/xenxs:Refactor graphics config parsing code

2014-07-10 Thread David Kiarie
From: Kiarie Kahurani davidkiar...@gmail.com

introduce function

  xenParseXMVfb(.);

which parses the graphics config

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

diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 80a7941..89d5739 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -872,125 +872,14 @@ static int xenParseXMDisk(virConfPtr conf, 
virDomainDefPtr def,
 
 return 0;
 }
-virDomainDefPtr
-xenParseXM(virConfPtr conf, int xendConfigVersion,
-   virCapsPtr caps)
+static int xenParseXMVfb(virConfPtr conf, virDomainDefPtr def,
+  int xendConfigVersion)
 {
-const char *str;
-int hvm = 0;
+char *listenAddr;
 int val;
-virConfValuePtr list;
-virDomainDefPtr def = NULL;
-virDomainDiskDefPtr disk = NULL;
-virDomainNetDefPtr net = NULL;
+virConfValuePtr list = NULL;
 virDomainGraphicsDefPtr graphics = NULL;
-size_t i;
-char *script = NULL;
-char *listenAddr = NULL;
-
-if (VIR_ALLOC(def)  0)
-return NULL;
-
-def-virtType = VIR_DOMAIN_VIRT_XEN;
-def-id = -1;
-if (xenParseXMGeneral(conf, def, caps)  0)
-goto cleanup;
-hvm = (STREQ(def-os.type, hvm));
-if (hvm) {
-const char *boot;
-if (xenXMConfigCopyString(conf, kernel, def-os.loader)  0)
-goto cleanup;
-
-if (xenXMConfigGetString(conf, boot, boot, c)  0)
-goto cleanup;
-
-for (i = 0; i  VIR_DOMAIN_BOOT_LAST  boot[i]; i++) {
-switch (*boot) {
-case 'a':
-def-os.bootDevs[i] = VIR_DOMAIN_BOOT_FLOPPY;
-break;
-case 'd':
-def-os.bootDevs[i] = VIR_DOMAIN_BOOT_CDROM;
-break;
-case 'n':
-def-os.bootDevs[i] = VIR_DOMAIN_BOOT_NET;
-break;
-case 'c':
-default:
-def-os.bootDevs[i] = VIR_DOMAIN_BOOT_DISK;
-break;
-}
-def-os.nBootDevs++;
-}
-} else {
-const char *extra, *root;
-
-if (xenXMConfigCopyStringOpt(conf, bootloader, def-os.bootloader) 
 0)
-goto cleanup;
-if (xenXMConfigCopyStringOpt(conf, bootargs, 
def-os.bootloaderArgs)  0)
-goto cleanup;
-
-if (xenXMConfigCopyStringOpt(conf, kernel, def-os.kernel)  0)
-goto cleanup;
-if (xenXMConfigCopyStringOpt(conf, ramdisk, def-os.initrd)  0)
-goto cleanup;
-if (xenXMConfigGetString(conf, extra, extra, NULL)  0)
-goto cleanup;
-if (xenXMConfigGetString(conf, root, root, NULL)  0)
-goto cleanup;
-
-if (root) {
-if (virAsprintf(def-os.cmdline, root=%s %s, root, extra)  0)
-goto cleanup;
-} else {
-if (VIR_STRDUP(def-os.cmdline, extra)  0)
-goto cleanup;
-}
-}
-if (xenParseXMMem(conf, def)  0)
-goto cleanup;
-if (xenXMConfigCopyStringOpt(conf, device_model, def-emulator)  0)
-goto cleanup;
-   if (xenParseXMVif(conf, def)  0)
-   goto cleanup;
-
-   if (xenParseXMTimeOffset(conf, def, xendConfigVersion)  0)
-   goto cleanup;
-   if (xenParseXMEventsActions(conf, def)  0)
-   goto cleanup;
-   if (xenParseXMPCI(conf, def)  0)
-   goto cleanup;
-   if (xenParseXMCPUFeatures(conf, def)  0)
-   goto cleanup;
-   if (xenParseXMDisk(conf, def, xendConfigVersion)  0)
-   goto cleanup;
-
-   if (hvm) {
-if (xenXMConfigGetString(conf, usbdevice, str, NULL)  0)
-goto cleanup;
-if (str 
-(STREQ(str, tablet) ||
- STREQ(str, mouse) ||
- STREQ(str, keyboard))) {
-virDomainInputDefPtr input;
-if (VIR_ALLOC(input)  0)
-goto cleanup;
-input-bus = VIR_DOMAIN_INPUT_BUS_USB;
-if (STREQ(str, mouse))
-input-type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
-else if (STREQ(str, tablet))
-input-type = VIR_DOMAIN_INPUT_TYPE_TABLET;
-else if (STREQ(str, keyboard))
-input-type = VIR_DOMAIN_INPUT_TYPE_KBD;
-if (VIR_ALLOC_N(def-inputs, 1)  0) {
-virDomainInputDefFree(input);
-goto cleanup;
-}
-def-inputs[0] = input;
-def-ninputs = 1;
-}
-}
-
+int hvm = STREQ(def-os.type, hvm);
 /* HVM guests, or old PV guests use this config format */
 if (hvm || xendConfigVersion  XEND_CONFIG_VERSION_3_0_4) {
 if (xenXMConfigGetBool(conf, vnc, val, 0)  0)
@@ -1125,6 +1014,132 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
 }
 }
 
+return 0;
+
+ cleanup:
+virDomainGraphicsDefFree(graphics);
+return -1;
+}

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

2014-07-10 Thread David Kiarie
From: Kiarie Kahurani davidkiar...@gmail.com

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

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

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

[libvirt] [PREPOST 09/17] src/xenxs:Refactor OS and Miscellaneous deivices config

2014-07-10 Thread David Kiarie
From: Kiarie Kahurani davidkiar...@gmail.com

Introduce functions
 xenParseXMOS(.) and
 xenParseXMMisc(.) to parse the OS and
Miscellaneous devices configs

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

diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 89d5739..312d890 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1020,36 +1020,21 @@ static int xenParseXMVfb(virConfPtr conf, 
virDomainDefPtr def,
 virDomainGraphicsDefFree(graphics);
 return -1;
 }
-virDomainDefPtr
-xenParseXM(virConfPtr conf, int xendConfigVersion,
-   virCapsPtr caps)
+static int xenParseXMOS(virConfPtr conf, virDomainDefPtr def)
 {
-const char *str;
-int hvm = 0;
-virConfValuePtr list;
-virDomainDefPtr def = NULL;
-virDomainDiskDefPtr disk = NULL;
-virDomainNetDefPtr net = NULL;
-virDomainGraphicsDefPtr graphics = NULL;
 size_t i;
-char *script = NULL;
-char *listenAddr = NULL;
-
-if (VIR_ALLOC(def)  0)
-return NULL;
 
-def-virtType = VIR_DOMAIN_VIRT_XEN;
-def-id = -1;
-if (xenParseXMGeneral(conf, def, caps)  0)
-goto cleanup;
-hvm = (STREQ(def-os.type, hvm));
-if (hvm) {
+if (STREQ(def-os.type, hvm)) {
 const char *boot;
+
+if (xenXMConfigCopyStringOpt(conf, device_model, def-emulator)  0)
+return -1;
+
 if (xenXMConfigCopyString(conf, kernel, def-os.loader)  0)
-goto cleanup;
+return -1;
 
 if (xenXMConfigGetString(conf, boot, boot, c)  0)
-goto cleanup;
+return -1;
 
 for (i = 0; i  VIR_DOMAIN_BOOT_LAST  boot[i]; i++) {
 switch (*boot) {
@@ -1073,55 +1058,52 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
 const char *extra, *root;
 
 if (xenXMConfigCopyStringOpt(conf, bootloader, def-os.bootloader) 
 0)
-goto cleanup;
+return -1;
 if (xenXMConfigCopyStringOpt(conf, bootargs, 
def-os.bootloaderArgs)  0)
-goto cleanup;
+return -1;
 
 if (xenXMConfigCopyStringOpt(conf, kernel, def-os.kernel)  0)
-goto cleanup;
+return -1;
 if (xenXMConfigCopyStringOpt(conf, ramdisk, def-os.initrd)  0)
-goto cleanup;
+return -1;
 if (xenXMConfigGetString(conf, extra, extra, NULL)  0)
-goto cleanup;
+return -1;
 if (xenXMConfigGetString(conf, root, root, NULL)  0)
-goto cleanup;
+return -1;
 
 if (root) {
 if (virAsprintf(def-os.cmdline, root=%s %s, root, extra)  0)
-goto cleanup;
+return -1;
 } else {
 if (VIR_STRDUP(def-os.cmdline, extra)  0)
-goto cleanup;
+return -1;
 }
 }
-if (xenParseXMMem(conf, def)  0)
-goto cleanup;
-if (xenXMConfigCopyStringOpt(conf, device_model, def-emulator)  0)
-goto cleanup;
-   if (xenParseXMVif(conf, def)  0)
-   goto cleanup;
-
-   if (xenParseXMTimeOffset(conf, def, xendConfigVersion)  0)
-   goto cleanup;
-   if (xenParseXMEventsActions(conf, def)  0)
-   goto cleanup;
-   if (xenParseXMPCI(conf, def)  0)
-   goto cleanup;
-   if (xenParseXMCPUFeatures(conf, def)  0)
-   goto cleanup;
-   if (xenParseXMDisk(conf, def, xendConfigVersion)  0)
-   goto cleanup;
-
-   if (hvm) {
+
+return 0;
+}
+static int xenParseXMMisc(virConfPtr conf, virDomainDefPtr def)
+{
+const char *str;
+
+if (STREQ(def-os.type, hvm)) {
+if (xenXMConfigGetString(conf, soundhw, str, NULL)  0)
+return -1;
+
+if (str 
+xenParseSxprSound(def, str)  0)
+return -1;
+
 if (xenXMConfigGetString(conf, usbdevice, str, NULL)  0)
-goto cleanup;
+return -1;
+
 if (str 
 (STREQ(str, tablet) ||
  STREQ(str, mouse) ||
  STREQ(str, keyboard))) {
 virDomainInputDefPtr input;
 if (VIR_ALLOC(input)  0)
-goto cleanup;
+return -1;
 input-bus = VIR_DOMAIN_INPUT_BUS_USB;
 if (STREQ(str, mouse))
 input-type = VIR_DOMAIN_INPUT_TYPE_MOUSE;
@@ -1131,15 +1113,61 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
 input-type = VIR_DOMAIN_INPUT_TYPE_KBD;
 if (VIR_ALLOC_N(def-inputs, 1)  0) {
 virDomainInputDefFree(input);
-goto cleanup;
+return -1;
 }
 def-inputs[0] = input;
 def-ninputs = 1;
 }
 }
-if (xenParseXMVfb(conf, def, xendConfigVersion)  0)
+
+return 0;
+}
+virDomainDefPtr
+xenParseXM(virConfPtr conf, int xendConfigVersion,
+   

[libvirt] [PREPOST 12/17] src/xenxs:Refactor virtual time config options formating code

2014-07-10 Thread David Kiarie
From: Kiarie Kahurani davidkiar...@gmail.com

Introduce function
 xenFormatXMTimeOffset(..)
which formats virtual time config

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

diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index a907252..c91fa73 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1634,12 +1634,88 @@ static int xenFormatXMMem(virConfPtr conf, 
virDomainDefPtr def)
 
 return 0;
 }
+static int xenFormatXMTimeOffset(virConfPtr conf, virDomainDefPtr def,
+ int xendConfigVersion)
+{
+int vmlocaltime;
+if (xendConfigVersion  XEND_CONFIG_VERSION_3_1_0) {
+/* 3.1: UTC and LOCALTIME */
+switch (def-clock.offset) {
+case VIR_DOMAIN_CLOCK_OFFSET_UTC:
+vmlocaltime = 0;
+break;
+case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
+vmlocaltime = 1;
+break;
+default:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(unsupported clock offset='%s'),
+   
virDomainClockOffsetTypeToString(def-clock.offset));
+return -1;
+}
+} else {
+if (STREQ(def-os.type, hvm)) {
+/* =3.1 HV: VARIABLE */
+int rtc_timeoffset;
+switch (def-clock.offset) {
+case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE:
+vmlocaltime = (int)def-clock.data.variable.basis;
+rtc_timeoffset = def-clock.data.variable.adjustment;
+break;
+case VIR_DOMAIN_CLOCK_OFFSET_UTC:
+if (def-clock.data.utc_reset) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(unsupported clock adjustment='reset'));
+return -1;
+}
+vmlocaltime = 0;
+rtc_timeoffset = 0;
+break;
+case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
+if (def-clock.data.utc_reset) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+   _(unsupported clock adjustment='reset'));
+return -1;
+}
+vmlocaltime = 1;
+rtc_timeoffset = 0;
+break;
+default:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(unsupported clock offset='%s'),
+   
virDomainClockOffsetTypeToString(def-clock.offset));
+return -1;
+}
+if (xenXMConfigSetInt(conf, rtc_timeoffset, rtc_timeoffset)  0)
+return -1;
+} else {
+/* =3.1 PV: UTC and LOCALTIME */
+switch (def-clock.offset) {
+case VIR_DOMAIN_CLOCK_OFFSET_UTC:
+vmlocaltime = 0;
+break;
+case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
+vmlocaltime = 1;
+break;
+default:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _(unsupported clock offset='%s'),
+   
virDomainClockOffsetTypeToString(def-clock.offset));
+return -1;
+}
+} /* !hvm */
+}
+if (xenXMConfigSetInt(conf, localtime, vmlocaltime)  0)
+return -1;
+
+return 0;
+}
 virConfPtr xenFormatXM(virConnectPtr conn,
virDomainDefPtr def,
int xendConfigVersion)
 {
 virConfPtr conf = NULL;
-int hvm = 0, vmlocaltime = 0;
+int hvm = 0;
 size_t i;
 char *cpus = NULL;
 const char *lifecycle;
@@ -1778,78 +1854,9 @@ virConfPtr xenFormatXM(virConnectPtr conn,
 goto cleanup;
 } /* !hvm */
 
-
-if (xendConfigVersion  XEND_CONFIG_VERSION_3_1_0) {
-/* 3.1: UTC and LOCALTIME */
-switch (def-clock.offset) {
-case VIR_DOMAIN_CLOCK_OFFSET_UTC:
-vmlocaltime = 0;
-break;
-case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
-vmlocaltime = 1;
-break;
-default:
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _(unsupported clock offset='%s'),
-   
virDomainClockOffsetTypeToString(def-clock.offset));
-goto cleanup;
-}
-} else {
-if (hvm) {
-/* =3.1 HV: VARIABLE */
-int rtc_timeoffset;
-switch (def-clock.offset) {
-case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE:
-vmlocaltime = (int)def-clock.data.variable.basis;
-rtc_timeoffset = def-clock.data.variable.adjustment;
-break;
-case VIR_DOMAIN_CLOCK_OFFSET_UTC:
-if 

[libvirt] [PREPOST 17/17] src/xenxs:Refactor disk config formating code

2014-07-10 Thread David Kiarie
From: Kiarie Kahurani davidkiar...@gmail.com

Introduce function
xenFormatXMDomainDisks(virConfPtr conf, .);
which formats disk config

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

diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 613730f..10f3d05 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1956,6 +1956,41 @@ static int xenFormatXMVfb(virConfPtr conf, 
virDomainDefPtr def,
 
 return 0;
 }
+static int xenFormatXMDomainDisks(virConfPtr conf, virDomainDefPtr def,
+  int xendConfigVersion)
+{
+virConfValuePtr diskVal = NULL;
+size_t i = 0;
+int hvm = STREQ(def-os.type, hvm);
+if (VIR_ALLOC(diskVal)  0)
+return -1;
+diskVal-type = VIR_CONF_LIST;
+diskVal-list = NULL;
+
+for (i = 0; i  def-ndisks; i++) {
+if (xendConfigVersion == XEND_CONFIG_VERSION_3_0_2 
+def-disks[i]-device == VIR_DOMAIN_DISK_DEVICE_CDROM 
+def-disks[i]-dst 
+STREQ(def-disks[i]-dst, hdc)) {
+continue;
+}
+if (def-disks[i]-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
+continue;
+
+if (xenFormatXMDisk(diskVal, def-disks[i],
+hvm, xendConfigVersion)  0)
+return -1;
+}
+if (diskVal-list != NULL) {
+int ret = virConfSetValue(conf, disk, diskVal);
+diskVal = NULL;
+if (ret  0)
+return -1;
+}
+VIR_FREE(diskVal);
+
+return 0;
+}
 virConfPtr xenFormatXM(virConnectPtr conn,
virDomainDefPtr def,
int xendConfigVersion)
@@ -2134,33 +2169,8 @@ virConfPtr xenFormatXM(virConnectPtr conn,
 }
 if (xenFormatXMVfb(conf, def, xendConfigVersion)  0)
 goto cleanup;
-/* analyze of the devices */
-if (VIR_ALLOC(diskVal)  0)
+if (xenFormatXMDomainDisks(conf, def, xendConfigVersion)  0)
 goto cleanup;
-diskVal-type = VIR_CONF_LIST;
-diskVal-list = NULL;
-
-for (i = 0; i  def-ndisks; i++) {
-if (xendConfigVersion == XEND_CONFIG_VERSION_3_0_2 
-def-disks[i]-device == VIR_DOMAIN_DISK_DEVICE_CDROM 
-def-disks[i]-dst 
-STREQ(def-disks[i]-dst, hdc)) {
-continue;
-}
-if (def-disks[i]-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
-continue;
-
-if (xenFormatXMDisk(diskVal, def-disks[i],
-hvm, xendConfigVersion)  0)
-goto cleanup;
-}
-if (diskVal-list != NULL) {
-int ret = virConfSetValue(conf, disk, diskVal);
-diskVal = NULL;
-if (ret  0)
-goto cleanup;
-}
-VIR_FREE(diskVal);
 if (xenFormatXMDomainNet(conf, conn, def, xendConfigVersion)  0)
 goto cleanup;
 if (xenFormatXMPCI(conf, def)  0)
-- 
1.8.4.5

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


[libvirt] [PREPOST 14/17] src/xenxs:Refactor code formating char devices config

2014-07-10 Thread David Kiarie
From: Kiarie Kahurani davidkiar...@gmail.com

Introduce function
 xenFormatXMCharDev();
which formats char devices config

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

diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 367a8cd..ee5dc19 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1742,6 +1742,85 @@ static int xenFormatXMEventActions(virConfPtr conf, 
virDomainDefPtr def)
 
 return 0;
 }
+static int xenFormatXMCharDev(virConfPtr conf, virDomainDefPtr def)
+{
+size_t i;
+if (STREQ(def-os.type, hvm)) {
+if (def-nparallels) {
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+char *str;
+int ret;
+
+ret = xenFormatSxprChr(def-parallels[0], buf);
+str = virBufferContentAndReset(buf);
+if (ret == 0)
+ret = xenXMConfigSetString(conf, parallel, str);
+VIR_FREE(str);
+if (ret  0)
+goto cleanup;
+} else {
+if (xenXMConfigSetString(conf, parallel, none)  0)
+goto cleanup;
+}
+
+if (def-nserials) {
+if ((def-nserials == 1)  (def-serials[0]-target.port == 0)) {
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+char *str;
+int ret;
+
+ret = xenFormatSxprChr(def-serials[0], buf);
+str = virBufferContentAndReset(buf);
+if (ret == 0)
+ret = xenXMConfigSetString(conf, serial, str);
+VIR_FREE(str);
+if (ret  0)
+goto cleanup;
+} else {
+size_t j = 0;
+int maxport = -1, port;
+virConfValuePtr serialVal = NULL;
+
+if (VIR_ALLOC(serialVal)  0)
+goto cleanup;
+serialVal-type = VIR_CONF_LIST;
+serialVal-list = NULL;
+
+for (i = 0; i  def-nserials; i++)
+if (def-serials[i]-target.port  maxport)
+maxport = def-serials[i]-target.port;
+
+for (port = 0; port = maxport; port++) {
+virDomainChrDefPtr chr = NULL;
+for (j = 0; j  def-nserials; j++) {
+if (def-serials[j]-target.port == port) {
+chr = def-serials[j];
+break;
+}
+}
+if (xenFormatXMSerial(serialVal, chr)  0) {
+virConfFreeValue(serialVal);
+goto cleanup;
+}
+}
+
+if (serialVal-list != NULL) {
+int ret = virConfSetValue(conf, serial, serialVal);
+serialVal = NULL;
+if (ret  0)
+goto cleanup;
+}
+VIR_FREE(serialVal);
+}
+} else {
+if (xenXMConfigSetString(conf, serial, none)  0)
+goto cleanup;
+}
+}
+return 0;
+cleanup:
+return -1;
+}
 virConfPtr xenFormatXM(virConnectPtr conn,
virDomainDefPtr def,
int xendConfigVersion)
@@ -2067,79 +2146,10 @@ virConfPtr xenFormatXM(virConnectPtr conn,
 if (xenFormatXMPCI(conf, def)  0)
 goto cleanup;
 
-if (hvm) {
-if (def-nparallels) {
-virBuffer buf = VIR_BUFFER_INITIALIZER;
-char *str;
-int ret;
-
-ret = xenFormatSxprChr(def-parallels[0], buf);
-str = virBufferContentAndReset(buf);
-if (ret == 0)
-ret = xenXMConfigSetString(conf, parallel, str);
-VIR_FREE(str);
-if (ret  0)
-goto cleanup;
-} else {
-if (xenXMConfigSetString(conf, parallel, none)  0)
-goto cleanup;
-}
-
-if (def-nserials) {
-if ((def-nserials == 1)  (def-serials[0]-target.port == 0)) {
-virBuffer buf = VIR_BUFFER_INITIALIZER;
-char *str;
-int ret;
-
-ret = xenFormatSxprChr(def-serials[0], buf);
-str = virBufferContentAndReset(buf);
-if (ret == 0)
-ret = xenXMConfigSetString(conf, serial, str);
-VIR_FREE(str);
-if (ret  0)
-goto cleanup;
-} else {
-size_t j = 0;
-int maxport = -1, port;
-virConfValuePtr serialVal = NULL;
-
-if (VIR_ALLOC(serialVal)  0)
-goto cleanup;
-serialVal-type = VIR_CONF_LIST;
-serialVal-list = 

[libvirt] [PREPOST 01/17] src/xenxs:Introduce function

2014-07-10 Thread David Kiarie
From: Kiarie Kahurani davidkiar...@gmail.com

Introduce function xenParseXMGeneral(virConfPtr conf, ...);
This function parses the xm general options such as uuid and name

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

diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 25a042d..1953a85 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -253,44 +253,25 @@ xenXMConfigGetUUID(virConfPtr conf, const char *name, 
unsigned char *uuid)
  * Turn a config record into a lump of XML describing the
  * domain, suitable for later feeding for virDomainCreateXML
  */
-virDomainDefPtr
-xenParseXM(virConfPtr conf, int xendConfigVersion,
-   virCapsPtr caps)
+static int xenParseXMGeneral(virConfPtr conf, virDomainDefPtr def,
+  virCapsPtr caps)
 {
+
+const char *defaultMachine;
 const char *str;
 int hvm = 0;
-int val;
-virConfValuePtr list;
-virDomainDefPtr def = NULL;
-virDomainDiskDefPtr disk = NULL;
-virDomainNetDefPtr net = NULL;
-virDomainGraphicsDefPtr graphics = NULL;
-virDomainHostdevDefPtr hostdev = NULL;
-size_t i;
-const char *defaultMachine;
-int vmlocaltime = 0;
-unsigned long count;
-char *script = NULL;
-char *listenAddr = NULL;
-
-if (VIR_ALLOC(def)  0)
-return NULL;
-
-def-virtType = VIR_DOMAIN_VIRT_XEN;
-def-id = -1;
 
 if (xenXMConfigCopyString(conf, name, def-name)  0)
-goto cleanup;
+return -1;
 if (xenXMConfigGetUUID(conf, uuid, def-uuid)  0)
-goto cleanup;
-
+return -1;
 
 if ((xenXMConfigGetString(conf, builder, str, linux) == 0) 
 STREQ(str, hvm))
 hvm = 1;
 
 if (VIR_STRDUP(def-os.type, hvm ? hvm : xen)  0)
-goto cleanup;
+return -1;
 
 def-os.arch =
 virCapabilitiesDefaultGuestArch(caps,
@@ -300,7 +281,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(no supported architecture for os type '%s'),
def-os.type);
-goto cleanup;
+return -1;
 }
 
 defaultMachine = virCapabilitiesDefaultGuestMachine(caps,
@@ -309,9 +290,37 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
 
virDomainVirtTypeToString(def-virtType));
 if (defaultMachine != NULL) {
 if (VIR_STRDUP(def-os.machine, defaultMachine)  0)
-goto cleanup;
+return -1;
 }
+return 0;
+}
+virDomainDefPtr
+xenParseXM(virConfPtr conf, int xendConfigVersion,
+   virCapsPtr caps)
+{
+const char *str;
+int hvm = 0;
+int val;
+virConfValuePtr list;
+virDomainDefPtr def = NULL;
+virDomainDiskDefPtr disk = NULL;
+virDomainNetDefPtr net = NULL;
+virDomainGraphicsDefPtr graphics = NULL;
+virDomainHostdevDefPtr hostdev = NULL;
+size_t i;
+int vmlocaltime = 0;
+unsigned long count;
+char *script = NULL;
+char *listenAddr = NULL;
+
+if (VIR_ALLOC(def)  0)
+return NULL;
 
+def-virtType = VIR_DOMAIN_VIRT_XEN;
+def-id = -1;
+if (xenParseXMGeneral(conf, def, caps)  0)
+goto cleanup;
+hvm = (STREQ(def-os.type, hvm));
 if (hvm) {
 const char *boot;
 if (xenXMConfigCopyString(conf, kernel, def-os.loader)  0)
-- 
1.8.4.5

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


[libvirt] [PREPOST 15/17] src/xenxs:Refactor Vif formating code

2014-07-10 Thread David Kiarie
From: Kiarie Kahurani davidkiar...@gmail.com

Introduce the function
 xenFormatXMDomainNet(..)
I think this could be done in a cleaner way

signed-of-by: David Kiariedavidkiar...@gmail.com
---
 src/xenxs/xen_xm.c | 49 +++--
 1 file changed, 31 insertions(+), 18 deletions(-)

diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index ee5dc19..8dd2823 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -1821,6 +1821,36 @@ static int xenFormatXMCharDev(virConfPtr conf, 
virDomainDefPtr def)
 cleanup:
 return -1;
 }
+static int xenFormatXMDomainNet(virConfPtr conf, virConnectPtr conn,
+ virDomainDefPtr def, int xendConfigVersion)
+{
+   virConfValuePtr netVal = NULL;
+   size_t i;
+
+   int hvm = STREQ(def-os.type, hvm);
+
+   if (VIR_ALLOC(netVal)  0)
+goto cleanup;
+netVal-type = VIR_CONF_LIST;
+netVal-list = NULL;
+
+for (i = 0; i  def-nnets; i++) {
+if (xenFormatXMNet(conn, netVal, def-nets[i],
+   hvm, xendConfigVersion)  0)
+   return -1;
+}
+if (netVal-list != NULL) {
+int ret = virConfSetValue(conf, vif, netVal);
+netVal = NULL;
+if (ret  0)
+return -1;
+}
+VIR_FREE(netVal);
+return 0;
+ cleanup:
+VIR_FREE(netVal);
+return -1;
+}
 virConfPtr xenFormatXM(virConnectPtr conn,
virDomainDefPtr def,
int xendConfigVersion)
@@ -2124,25 +2154,8 @@ virConfPtr xenFormatXM(virConnectPtr conn,
 goto cleanup;
 }
 VIR_FREE(diskVal);
-
-if (VIR_ALLOC(netVal)  0)
+if (xenFormatXMDomainNet(conf, conn, def, xendConfigVersion)  0)
 goto cleanup;
-netVal-type = VIR_CONF_LIST;
-netVal-list = NULL;
-
-for (i = 0; i  def-nnets; i++) {
-if (xenFormatXMNet(conn, netVal, def-nets[i],
-   hvm, xendConfigVersion)  0)
-goto cleanup;
-}
-if (netVal-list != NULL) {
-int ret = virConfSetValue(conf, vif, netVal);
-netVal = NULL;
-if (ret  0)
-goto cleanup;
-}
-VIR_FREE(netVal);
-
 if (xenFormatXMPCI(conf, def)  0)
 goto cleanup;
 
-- 
1.8.4.5

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


[libvirt] [PATCH v2 2/4] virSecurityDeviceLabelDef: substitute 'norelabel' with 'relabel'

2014-07-10 Thread Michal Privoznik
Similarly to the previous commit, boolean variables should not start
with 'no-' prefix.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/conf/domain_conf.c  | 12 ++--
 src/security/security_dac.c |  8 
 src/security/security_selinux.c | 10 +-
 src/util/virseclabel.c  |  2 +-
 src/util/virseclabel.h  |  2 +-
 5 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8384b5d..eccecd4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4803,9 +4803,9 @@ 
virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn,
 relabel = virXMLPropString(list[i], relabel);
 if (relabel != NULL) {
 if (STREQ(relabel, yes)) {
-seclabels[i]-norelabel = false;
+seclabels[i]-relabel = true;
 } else if (STREQ(relabel, no)) {
-seclabels[i]-norelabel = true;
+seclabels[i]-relabel = false;
 } else {
 virReportError(VIR_ERR_XML_ERROR,
_(invalid security relabel value %s),
@@ -4815,7 +4815,7 @@ 
virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn,
 }
 VIR_FREE(relabel);
 } else {
-seclabels[i]-norelabel = false;
+seclabels[i]-relabel = true;
 }
 
 /* labelskip is only parsed on live images */
@@ -4830,7 +4830,7 @@ 
virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn,
 VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
 seclabels[i]-label = label;
 
-if (label  seclabels[i]-norelabel) {
+if (label  !seclabels[i]-relabel) {
 virReportError(VIR_ERR_XML_ERROR,
_(Cannot specify a label if relabelling is 
  turned off. model=%s),
@@ -14736,7 +14736,7 @@ virSecurityDeviceLabelDefFormat(virBufferPtr buf,
 {
 /* For offline output, skip elements that allow labels but have no
  * label specified (possible if labelskip was ignored on input).  */
-if ((flags  VIR_DOMAIN_XML_INACTIVE)  !def-label  !def-norelabel)
+if ((flags  VIR_DOMAIN_XML_INACTIVE)  !def-label  def-relabel)
 return;
 
 virBufferAddLit(buf, seclabel);
@@ -14747,7 +14747,7 @@ virSecurityDeviceLabelDefFormat(virBufferPtr buf,
 if (def-labelskip)
 virBufferAddLit(buf,  labelskip='yes');
 else
-virBufferAsprintf(buf,  relabel='%s', def-norelabel ? no : yes);
+virBufferAsprintf(buf,  relabel='%s', def-relabel ? yes : no);
 
 if (def-label) {
 virBufferAddLit(buf, \n);
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 665cbc9..4d2a9d6 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -312,7 +312,7 @@ virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr 
mgr,
 
 disk_seclabel = virStorageSourceGetSecurityLabelDef(src,
 SECURITY_DAC_NAME);
-if (disk_seclabel  disk_seclabel-norelabel)
+if (disk_seclabel  !disk_seclabel-relabel)
 return 0;
 
 if (disk_seclabel  disk_seclabel-label) {
@@ -374,7 +374,7 @@ 
virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr,
 
 disk_seclabel = virStorageSourceGetSecurityLabelDef(src,
 SECURITY_DAC_NAME);
-if (disk_seclabel  disk_seclabel-norelabel)
+if (disk_seclabel  !disk_seclabel-relabel)
 return 0;
 
 /* If we have a shared FS and are doing migration, we must not change
@@ -703,7 +703,7 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
 chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
   SECURITY_DAC_NAME);
 
-if (chr_seclabel  chr_seclabel-norelabel)
+if (chr_seclabel  !chr_seclabel-relabel)
 return 0;
 
 if (chr_seclabel  chr_seclabel-label) {
@@ -772,7 +772,7 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr 
ATTRIBUTE_UNUSED,
 chr_seclabel = virDomainChrDefGetSecurityLabelDef(dev,
   SECURITY_DAC_NAME);
 
-if (chr_seclabel  chr_seclabel-norelabel)
+if (chr_seclabel  !chr_seclabel-relabel)
 return 0;
 
 switch ((virDomainChrType) dev_source-type) {
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 8d4a9aa..a0e89b7 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1130,7 +1130,7 @@ 
virSecuritySELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr,
 
 disk_seclabel = virStorageSourceGetSecurityLabelDef(src,
 SECURITY_SELINUX_NAME);
-if (!seclabel-relabel || 

[libvirt] [PATCH v2 4/4] conf: Don't allow multiple seclabels for same model

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

With current code it's possible to have for instance:

virsh dumpxml mydomain | grep seclabel
  seclabel type='dynamic' model='selinux' relabel='yes'/
  seclabel type='dynamic' model='selinux' relabel='yes'/
  seclabel type='dynamic' model='selinux' relabel='yes'/
  seclabel type='dynamic' model='selinux' relabel='yes'/
  seclabel type='dynamic' model='selinux' relabel='yes'/

what doesn't make any sense. We should reject the XML in the config
parsing phase.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/conf/domain_conf.c | 18 --
 .../qemuxml2argv-seclabel-multiple.xml | 40 ++
 tests/qemuxml2argvtest.c   |  1 +
 3 files changed, 56 insertions(+), 3 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-multiple.xml

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c730d37..ace3ddf 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4668,7 +4668,7 @@ virSecurityLabelDefsParseXML(virDomainDefPtr def,
  virCapsPtr caps,
  unsigned int flags)
 {
-size_t i = 0;
+size_t i = 0, j;
 int n;
 xmlNodePtr *list = NULL, saved_node;
 virCapsHostPtr host = caps-host;
@@ -4689,10 +4689,22 @@ virSecurityLabelDefsParseXML(virDomainDefPtr def,
 
 /* Parse each seclabel tag */
 for (i = 0; i  n; i++) {
+virSecurityLabelDefPtr seclabel;
+
 ctxt-node = list[i];
-def-seclabels[i] = virSecurityLabelDefParseXML(ctxt, flags);
-if (def-seclabels[i] == NULL)
+if (!(seclabel = virSecurityLabelDefParseXML(ctxt, flags)))
 goto error;
+
+for (j = 0; j  i; j++) {
+if (STREQ_NULLABLE(seclabel-model, def-seclabels[j]-model)) {
+virReportError(VIR_ERR_XML_DETAIL,
+   _(seclablel for model %s is already provided),
+   seclabel-model);
+goto error;
+}
+}
+
+def-seclabels[i] = seclabel;
 }
 def-nseclabels = n;
 ctxt-node = saved_node;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-multiple.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-multiple.xml
new file mode 100644
index 000..bd6fd15
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-multiple.xml
@@ -0,0 +1,40 @@
+domain type='qemu' id='1'
+  nameQEMUGuest1/name
+  uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid
+  memory unit='KiB'219100/memory
+  currentMemory unit='KiB'219100/currentMemory
+  vcpu placement='static' cpuset='1-4,8-20,525'1/vcpu
+  os
+type arch='i686' machine='pc'hvm/type
+boot dev='hd'/
+  /os
+  clock offset='utc'/
+  on_poweroffdestroy/on_poweroff
+  on_rebootrestart/on_reboot
+  on_crashdestroy/on_crash
+  devices
+emulator/usr/bin/qemu/emulator
+disk type='block' device='disk'
+  source dev='/dev/HostVG/QEMUGuest1'
+seclabel model='selinux' labelskip='yes'/
+  /source
+  backingStore/
+  target dev='hda' bus='ide'/
+  address type='drive' controller='0' bus='0' target='0' unit='0'/
+/disk
+controller type='usb' index='0'/
+controller type='ide' index='0'/
+controller type='pci' index='0' model='pci-root'/
+memballoon model='virtio'/
+  /devices
+  seclabel type='none' relabel='no'/
+  seclabel type='dynamic' model='dac' relabel='yes'/
+  seclabel type='static' model='selinux' relabel='yes'
+labelsystem_u:system_r:svirt_custom_t:s0:c192,c392/label
+imagelabelsystem_u:system_r:svirt_custom_t:s0:c192,c392/imagelabel
+  /seclabel
+  seclabel type='static' model='selinux' relabel='yes'
+labelsystem_u:system_r:svirt_custom_t:s0:c192,c393/label
+imagelabelsystem_u:system_r:svirt_custom_t:s0:c192,c393/imagelabel
+  /seclabel
+/domain
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index bbc0fb7..a841adb 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1223,6 +1223,7 @@ mymain(void)
 DO_TEST(seclabel-static-labelskip, QEMU_CAPS_NAME);
 DO_TEST(seclabel-none, QEMU_CAPS_NAME);
 DO_TEST(seclabel-dac-none, QEMU_CAPS_NAME);
+DO_TEST_PARSE_ERROR(seclabel-multiple, QEMU_CAPS_NAME);
 
 DO_TEST(pseries-basic,
 QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG);
-- 
1.8.5.5

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


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

2014-07-10 Thread Michal Privoznik
diff to v1:
- rework the 3rd patch
- introduce one more bugfix

Michal Privoznik (4):
  virSecurityLabelDef: substitute 'norelabel' with 'relabel'
  virSecurityDeviceLabelDef: substitute 'norelabel' with 'relabel'
  conf: Always format seclabel's model
  conf: Don't allow multiple seclabels for same model

 src/conf/domain_conf.c | 67 --
 src/security/security_apparmor.c   | 10 ++--
 src/security/security_dac.c| 22 +++
 src/security/security_manager.c|  2 +-
 src/security/security_selinux.c| 32 +--
 src/util/virseclabel.c |  2 +-
 src/util/virseclabel.h |  4 +-
 .../qemuxml2argv-seclabel-dynamic-none.xml | 28 +
 .../qemuxml2argv-seclabel-multiple.xml | 40 +
 tests/qemuxml2argvtest.c   |  1 +
 tests/qemuxml2xmltest.c|  1 +
 11 files changed, 142 insertions(+), 67 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-none.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-multiple.xml

-- 
1.8.5.5

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


[libvirt] [PATCH v2 1/4] virSecurityLabelDef: substitute 'norelabel' with 'relabel'

2014-07-10 Thread Michal Privoznik
This negation in names of boolean variables is driving me insane. The
code is much more readable if we drop the 'no-' prefix. Well, at least
for me.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/conf/domain_conf.c   | 20 ++--
 src/security/security_apparmor.c | 10 +-
 src/security/security_dac.c  | 14 +++---
 src/security/security_manager.c  |  2 +-
 src/security/security_selinux.c  | 24 ++--
 src/util/virseclabel.h   |  2 +-
 6 files changed, 34 insertions(+), 38 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index fa76eb4..8384b5d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4576,9 +4576,9 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt,
 VIR_SECURITY_LABEL_BUFLEN-1, ctxt);
 if (p != NULL) {
 if (STREQ(p, yes)) {
-def-norelabel = false;
+def-relabel = true;
 } else if (STREQ(p, no)) {
-def-norelabel = true;
+def-relabel = false;
 } else {
 virReportError(VIR_ERR_XML_ERROR,
_(invalid security relabel value %s), p);
@@ -4587,13 +4587,13 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt,
 }
 VIR_FREE(p);
 if (def-type == VIR_DOMAIN_SECLABEL_DYNAMIC 
-def-norelabel) {
+!def-relabel) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
%s, _(dynamic label type must use resource 
relabeling));
 goto error;
 }
 if (def-type == VIR_DOMAIN_SECLABEL_NONE 
-!def-norelabel) {
+def-relabel) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
%s, _(resource relabeling is not compatible with 
'none' label type));
 goto error;
@@ -4601,9 +4601,9 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt,
 } else {
 if (def-type == VIR_DOMAIN_SECLABEL_STATIC ||
 def-type == VIR_DOMAIN_SECLABEL_NONE)
-def-norelabel = true;
+def-relabel = false;
 else
-def-norelabel = false;
+def-relabel = true;
 }
 
 /* Always parse model */
@@ -4635,7 +4635,7 @@ virSecurityLabelDefParseXML(xmlXPathContextPtr ctxt,
 }
 
 /* Only parse imagelabel, if requested live XML with relabeling */
-if (!def-norelabel 
+if (def-relabel 
 (!(flags  VIR_DOMAIN_XML_INACTIVE) 
  def-type != VIR_DOMAIN_SECLABEL_NONE)) {
 p = virXPathStringLimit(string(./imagelabel[1]),
@@ -4793,7 +4793,7 @@ 
virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn,
 }
 
 /* Can't use overrides if top-level doesn't allow relabeling.  */
-if (vmDef  vmDef-norelabel) {
+if (vmDef  !vmDef-relabel) {
 virReportError(VIR_ERR_XML_ERROR, %s,
_(label overrides require relabeling to be 
  enabled at the domain level));
@@ -14708,14 +14708,14 @@ virSecurityLabelDefFormat(virBufferPtr buf,
 }
 
 virBufferAsprintf(buf,  relabel='%s',
-  def-norelabel ? no : yes);
+  def-relabel ? yes : no);
 
 if (def-label || def-imagelabel || def-baselabel) {
 virBufferAddLit(buf, \n);
 virBufferAdjustIndent(buf, 2);
 virBufferEscapeString(buf, label%s/label\n,
   def-label);
-if (!def-norelabel)
+if (def-relabel)
 virBufferEscapeString(buf, imagelabel%s/imagelabel\n,
   def-imagelabel);
 if (def-type == VIR_DOMAIN_SECLABEL_DYNAMIC)
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index 1e2a38b..9603c78 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -281,7 +281,7 @@ reload_profile(virSecurityManagerPtr mgr,
 if (!secdef)
 return rc;
 
-if (secdef-norelabel)
+if (!secdef-relabel)
 return 0;
 
 if ((profile_name = get_profile_name(def)) == NULL)
@@ -481,7 +481,7 @@ AppArmorSetSecurityAllLabel(virSecurityManagerPtr mgr,
 if (!secdef)
 return -1;
 
-if (secdef-norelabel)
+if (!secdef-relabel)
 return 0;
 
 /* Reload the profile if stdin_path is specified. Note that
@@ -718,7 +718,7 @@ AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr,
 if (!(secdef = virDomainDefGetSecurityLabelDef(def, 
SECURITY_APPARMOR_NAME)))
 return -1;
 
-if (secdef-norelabel)
+if (!secdef-relabel)
 return 0;
 
 if (secdef-imagelabel) {
@@ -805,7 +805,7 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr,
 if (!secdef)
 return -1;
 
-if (secdef-norelabel)
+if (!secdef-relabel)
 return 0;
 
 if (dev-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
@@ 

[libvirt] [PATCH v2 3/4] conf: Always format seclabel's model

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

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

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

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index eccecd4..c730d37 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14677,8 +14677,7 @@ virDomainEventActionDefFormat(virBufferPtr buf,
 
 static void
 virSecurityLabelDefFormat(virBufferPtr buf,
-  virSecurityLabelDefPtr def,
-  unsigned flags)
+  virSecurityLabelDefPtr def)
 {
 const char *sectype = virDomainSeclabelTypeToString(def-type);
 
@@ -14688,19 +14687,17 @@ virSecurityLabelDefFormat(virBufferPtr buf,
 if (def-type == VIR_DOMAIN_SECLABEL_DEFAULT)
 return;
 
-/* To avoid backward compatibility issues, suppress DAC labels that are
- * automatically generated.
+/* To avoid backward compatibility issues, suppress DAC and 'none' labels
+ * that are automatically generated.
  */
-if (STREQ_NULLABLE(def-model, dac)  def-implicit)
+if ((STREQ_NULLABLE(def-model, dac) ||
+ STREQ_NULLABLE(def-model, none))  def-implicit)
 return;
 
 virBufferAsprintf(buf, seclabel type='%s',
   sectype);
 
-/* When generating state XML do include the model */
-if (flags  VIR_DOMAIN_XML_INTERNAL_STATUS ||
-STRNEQ_NULLABLE(def-model, none))
-virBufferEscapeString(buf,  model='%s', def-model);
+virBufferEscapeString(buf,  model='%s', def-model);
 
 if (def-type == VIR_DOMAIN_SECLABEL_NONE) {
 virBufferAddLit(buf, /\n);
@@ -17910,7 +17907,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
 virBufferAddLit(buf, /devices\n);
 
 for (n = 0; n  def-nseclabels; n++)
-virSecurityLabelDefFormat(buf, def-seclabels[n], flags);
+virSecurityLabelDefFormat(buf, def-seclabels[n]);
 
 if (def-namespaceData  def-ns.format) {
 if ((def-ns.format)(buf, def-namespaceData)  0)
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-none.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-none.xml
new file mode 100644
index 000..7949f99
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-none.xml
@@ -0,0 +1,28 @@
+domain type='qemu'
+  nameQEMUGuest1/name
+  uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid
+  memory unit='KiB'219100/memory
+  currentMemory unit='KiB'219100/currentMemory
+  vcpu placement='static' cpuset='1-4,8-20,525'1/vcpu
+  os
+type arch='i686' machine='pc'hvm/type
+boot dev='hd'/
+  /os
+  clock offset='utc'/
+  on_poweroffdestroy/on_poweroff
+  on_rebootrestart/on_reboot
+  on_crashdestroy/on_crash
+  devices
+emulator/usr/bin/qemu/emulator
+disk type='block' device='disk'
+  source dev='/dev/HostVG/QEMUGuest1'/
+  target dev='hda' bus='ide'/
+  address type='drive' controller='0' bus='0' target='0' unit='0'/
+/disk
+controller type='usb' index='0'/
+controller type='ide' index='0'/
+controller type='pci' index='0' model='pci-root'/
+memballoon model='virtio'/
+  /devices
+  seclabel type='dynamic' model='none' relabel='yes'/
+/domain
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 26e3cad..9f919de 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -307,6 +307,7 @@ mymain(void)
 DO_TEST_FULL(seclabel-static-labelskip, false, WHEN_ACTIVE);
 DO_TEST(seclabel-none);
 DO_TEST(seclabel-dac-none);
+DO_TEST(seclabel-dynamic-none);
 DO_TEST(numad-static-vcpu-no-numatune);
 DO_TEST(disk-scsi-lun-passthrough-sgio);
 
-- 
1.8.5.5

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


[libvirt] [PATCH 7/8] storage: Implement virStorageFileCreate for local and gluster files

2014-07-10 Thread Peter Krempa
Add backends for this frontend function so that we can use it in the
snapshot creation code.
---
 src/storage/storage_backend_fs.c  | 17 +
 src/storage/storage_backend_gluster.c | 15 +++
 2 files changed, 32 insertions(+)

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 693ebd3..1705a5a 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1367,6 +1367,22 @@ virStorageFileBackendFileInit(virStorageSourcePtr src)


 static int
+virStorageFileBackendFileCreate(virStorageSourcePtr src)
+{
+int fd = -1;
+
+if ((fd = virFileOpenAs(src-path, O_WRONLY | O_TRUNC | O_CREAT, 0,
+src-drv-uid, src-drv-gid, 0))  0) {
+errno = -fd;
+return -1;
+}
+
+VIR_FORCE_CLOSE(fd);
+return 0;
+}
+
+
+static int
 virStorageFileBackendFileUnlink(virStorageSourcePtr src)
 {
 return unlink(src-path);
@@ -1450,6 +1466,7 @@ virStorageFileBackend virStorageFileBackendFile = {
 .backendInit = virStorageFileBackendFileInit,
 .backendDeinit = virStorageFileBackendFileDeinit,

+.storageFileCreate = virStorageFileBackendFileCreate,
 .storageFileUnlink = virStorageFileBackendFileUnlink,
 .storageFileStat = virStorageFileBackendFileStat,
 .storageFileReadHeader = virStorageFileBackendFileReadHeader,
diff --git a/src/storage/storage_backend_gluster.c 
b/src/storage/storage_backend_gluster.c
index edb0511..c2eafd4 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -626,6 +626,20 @@ virStorageFileBackendGlusterInit(virStorageSourcePtr src)


 static int
+virStorageFileBackendGlusterCreate(virStorageSourcePtr src)
+{
+virStorageFileBackendGlusterPrivPtr priv = src-drv-priv;
+glfs_fd_t *fd = NULL;
+
+if (!(fd = glfs_open(priv-vol, src-path, O_CREAT | O_TRUNC | O_WRONLY)))
+return -1;
+
+ignore_value(glfs_close(fd));
+return 0;
+}
+
+
+static int
 virStorageFileBackendGlusterUnlink(virStorageSourcePtr src)
 {
 virStorageFileBackendGlusterPrivPtr priv = src-drv-priv;
@@ -772,6 +786,7 @@ virStorageFileBackend virStorageFileBackendGluster = {
 .backendInit = virStorageFileBackendGlusterInit,
 .backendDeinit = virStorageFileBackendGlusterDeinit,

+.storageFileCreate = virStorageFileBackendGlusterCreate,
 .storageFileUnlink = virStorageFileBackendGlusterUnlink,
 .storageFileStat = virStorageFileBackendGlusterStat,
 .storageFileReadHeader = virStorageFileBackendGlusterReadHeader,
-- 
2.0.0

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


[libvirt] [PATCH 8/8] qemu: snapshot: Use storage driver to pre-create snapshot file

2014-07-10 Thread Peter Krempa
Move the last operation done on local files to the storage driver API.
---
 src/qemu/qemu_driver.c | 17 +++--
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9b3c695..a0f49eb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12881,7 +12881,6 @@ 
qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
 char *source = NULL;
 const char *formatStr = NULL;
 int ret = -1;
-int fd = -1;
 bool need_unlink = false;

 if (snap-snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
@@ -12899,7 +12898,7 @@ 
qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
 if (virStorageSourceInitChainElement(newDiskSrc, disk-src, false)  0)
 goto cleanup;

-if (virStorageFileInit(newDiskSrc)  0)
+if (qemuDomainStorageFileInit(driver, vm, newDiskSrc)  0)
 goto cleanup;

 if (qemuGetDriveSourceString(newDiskSrc, NULL, source)  0)
@@ -12915,15 +12914,13 @@ 
qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
 }

 /* pre-create the image file so that we can label it before handing it to 
qemu */
-/* XXX we should switch to storage driver based pre-creation of the image 
*/
-if (virStorageSourceIsLocalStorage(newDiskSrc)) {
-if (!reuse  newDiskSrc-type != VIR_STORAGE_TYPE_BLOCK) {
-fd = qemuOpenFile(driver, vm, source, O_WRONLY | O_TRUNC | O_CREAT,
-  need_unlink, NULL);
-if (fd  0)
-goto cleanup;
-VIR_FORCE_CLOSE(fd);
+if (!reuse  newDiskSrc-type != VIR_STORAGE_TYPE_BLOCK) {
+if (virStorageFileCreate(newDiskSrc)  0) {
+virReportSystemError(errno, _(failed to create image file '%s'),
+ source);
+goto cleanup;
 }
+need_unlink = true;
 }

 /* set correct security, cgroup and locking options on the new image */
-- 
2.0.0

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


[libvirt] [PATCH 4/8] security: DAC: Introduce callback to perform image chown

2014-07-10 Thread Peter Krempa
To integrate the security driver with the storage driver we need to
pass a callback for a function that will chown storage volumes.

Introduce and document the callback prototype.
---
 src/qemu/qemu_driver.c  |  3 ++-
 src/security/security_dac.c |  9 +
 src/security/security_dac.h |  3 +++
 src/security/security_manager.c |  4 +++-
 src/security/security_manager.h | 19 ++-
 5 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ecccf6c..b30c504 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -374,7 +374,8 @@ qemuSecurityInit(virQEMUDriverPtr driver)
  cfg-allowDiskFormatProbing,
  cfg-securityDefaultConfined,
  cfg-securityRequireConfined,
- cfg-dynamicOwnership)))
+ cfg-dynamicOwnership,
+ NULL)))
 goto error;
 if (!stack) {
 if (!(stack = virSecurityManagerNewStack(mgr)))
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 6821d37..eafd714 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -51,6 +51,7 @@ struct _virSecurityDACData {
 int ngroups;
 bool dynamicOwnership;
 char *baselabel;
+virSecurityManagerDACChownCallback chownCallback;
 };

 typedef struct _virSecurityDACCallbackData virSecurityDACCallbackData;
@@ -87,6 +88,14 @@ virSecurityDACSetDynamicOwnership(virSecurityManagerPtr mgr,
 priv-dynamicOwnership = dynamicOwnership;
 }

+void
+virSecurityDACSetChownCallback(virSecurityManagerPtr mgr,
+   virSecurityManagerDACChownCallback 
chownCallback)
+{
+virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+priv-chownCallback = chownCallback;
+}
+
 /* returns 1 if label isn't found, 0 on success, -1 on error */
 static int
 ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
diff --git a/src/security/security_dac.h b/src/security/security_dac.h
index dbcf56f..846cefb 100644
--- a/src/security/security_dac.h
+++ b/src/security/security_dac.h
@@ -32,4 +32,7 @@ int virSecurityDACSetUserAndGroup(virSecurityManagerPtr mgr,
 void virSecurityDACSetDynamicOwnership(virSecurityManagerPtr mgr,
bool dynamic);

+void virSecurityDACSetChownCallback(virSecurityManagerPtr mgr,
+virSecurityManagerDACChownCallback 
chownCallback);
+
 #endif /* __VIR_SECURITY_DAC */
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index 16bec5c..320dde4 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -152,7 +152,8 @@ virSecurityManagerNewDAC(const char *virtDriver,
  bool allowDiskFormatProbing,
  bool defaultConfined,
  bool requireConfined,
- bool dynamicOwnership)
+ bool dynamicOwnership,
+ virSecurityManagerDACChownCallback chownCallback)
 {
 virSecurityManagerPtr mgr =
 virSecurityManagerNewDriver(virSecurityDriverDAC,
@@ -170,6 +171,7 @@ virSecurityManagerNewDAC(const char *virtDriver,
 }

 virSecurityDACSetDynamicOwnership(mgr, dynamicOwnership);
+virSecurityDACSetChownCallback(mgr, chownCallback);

 return mgr;
 }
diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index 97b6a2e..156f882 100644
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -25,6 +25,7 @@

 # include domain_conf.h
 # include vircommand.h
+# include virstoragefile.h

 typedef struct _virSecurityManager virSecurityManager;
 typedef virSecurityManager *virSecurityManagerPtr;
@@ -39,13 +40,29 @@ virSecurityManagerPtr 
virSecurityManagerNewStack(virSecurityManagerPtr primary);
 int virSecurityManagerStackAddNested(virSecurityManagerPtr stack,
  virSecurityManagerPtr nested);

+/**
+ * virSecurityManagerDACChownCallback:
+ * @src: Storage file to chown
+ * @uid: target uid
+ * @gid: target gid
+ *
+ * A function callback to chown image files described by the disk source struct
+ * @src. The callback shall return 0 on success, -1 on error and errno set (no
+ * libvirt error reported) OR -2 and a libvirt error reported. */
+typedef int
+(*virSecurityManagerDACChownCallback)(virStorageSourcePtr src,
+  uid_t uid,
+  gid_t gid);
+
+
 virSecurityManagerPtr virSecurityManagerNewDAC(const char *virtDriver,
uid_t user,
gid_t group,

[libvirt] [PATCH 2/8] storage: Add witness for checking storage volume use in security driver

2014-07-10 Thread Peter Krempa
With my intended use of storage driver assist to chown files on remote
storage we will need a witness that will tell us whether the given
storage volume supports operations needed by the storage driver.
---
 src/storage/storage_driver.c | 30 ++
 src/storage/storage_driver.h |  2 ++
 2 files changed, 32 insertions(+)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 928fadf..fc05a08 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2567,6 +2567,36 @@ 
virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src)
backend-storageFileAccess;
 }

+
+/**
+ * virStorageFileSupportsSecurityDriver:
+ *
+ * @src: a storage file structure
+ *
+ * Check if a storage file supports operations needed by the security
+ * driver to perform labelling */
+bool
+virStorageFileSupportsSecurityDriver(virStorageSourcePtr src)
+{
+int actualType = virStorageSourceGetActualType(src);
+virStorageFileBackendPtr backend;
+
+if (!src)
+return false;
+
+if (src-drv) {
+backend = src-drv-backend;
+} else {
+if (!(backend = virStorageFileBackendForTypeInternal(actualType,
+ src-protocol,
+ false)))
+return false;
+}
+
+return !!backend-storageFileChown;
+}
+
+
 void
 virStorageFileDeinit(virStorageSourcePtr src)
 {
diff --git a/src/storage/storage_driver.h b/src/storage/storage_driver.h
index eefd766..9592dd8 100644
--- a/src/storage/storage_driver.h
+++ b/src/storage/storage_driver.h
@@ -45,6 +45,8 @@ const char 
*virStorageFileGetUniqueIdentifier(virStorageSourcePtr src);
 int virStorageFileAccess(virStorageSourcePtr src, int mode);
 int virStorageFileChown(virStorageSourcePtr src, uid_t uid, gid_t gid);

+bool virStorageFileSupportsSecurityDriver(virStorageSourcePtr src);
+
 int virStorageFileGetMetadata(virStorageSourcePtr src,
   uid_t uid, gid_t gid,
   bool allow_probe)
-- 
2.0.0

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


[libvirt] [PATCH 6/8] qemu: Implement DAC driver chown callback to co-operate with storage drv

2014-07-10 Thread Peter Krempa
Use the storage driver to chown remote images.
---
 src/qemu/qemu_driver.c | 48 +++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b30c504..9b3c695 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -327,6 +327,52 @@ qemuAutostartDomains(virQEMUDriverPtr driver)
 virObjectUnref(cfg);
 }

+
+static int
+qemuSecurityChownCallback(virStorageSourcePtr src,
+  uid_t uid,
+  gid_t gid)
+{
+struct stat sb;
+int save_errno = 0;
+int ret = -1;
+
+if (!virStorageFileSupportsSecurityDriver(src))
+return 0;
+
+if (virStorageSourceIsLocalStorage(src)) {
+/* use direct chmod for local files so that the file doesn't
+ * need to be initialized */
+if (stat(src-path, sb) = 0) {
+if (sb.st_uid == uid 
+sb.st_gid == gid) {
+/* It's alright, there's nothing to change anyway. */
+return 0;
+}
+}
+
+return chown(src-path, uid, gid);
+}
+
+/* storage file init reports errors, return -2 on failure */
+if (virStorageFileInit(src)  0)
+return -2;
+
+if (virStorageFileChown(src, uid, gid)  0) {
+save_errno = errno;
+goto cleanup;
+}
+
+ret = 0;
+
+ cleanup:
+virStorageFileDeinit(src);
+errno = save_errno;
+
+return ret;
+}
+
+
 static int
 qemuSecurityInit(virQEMUDriverPtr driver)
 {
@@ -375,7 +421,7 @@ qemuSecurityInit(virQEMUDriverPtr driver)
  cfg-securityDefaultConfined,
  cfg-securityRequireConfined,
  cfg-dynamicOwnership,
- NULL)))
+ qemuSecurityChownCallback)))
 goto error;
 if (!stack) {
 if (!(stack = virSecurityManagerNewStack(mgr)))
-- 
2.0.0

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


[libvirt] [PATCH 0/8] Add support for setting DAC permissions on remote filesystems

2014-07-10 Thread Peter Krempa
Peter Krempa (8):
  storage: Implement storage driver helper to chown disk images
  storage: Add witness for checking storage volume use in security
driver
  security: DAC: Remove superfluous link resolution
  security: DAC: Introduce callback to perform image chown
  security: DAC: Plumb usage of chown callback
  qemu: Implement DAC driver chown callback to co-operate with storage
drv
  storage: Implement virStorageFileCreate for local and gluster files
  qemu: snapshot: Use storage driver to pre-create snapshot file

 src/qemu/qemu_driver.c|  66 
 src/security/security_dac.c   | 109 +++---
 src/security/security_dac.h   |   3 +
 src/security/security_manager.c   |   4 +-
 src/security/security_manager.h   |  19 +-
 src/storage/storage_backend.h |   6 ++
 src/storage/storage_backend_fs.c  |  29 +
 src/storage/storage_backend_gluster.c |  27 +
 src/storage/storage_driver.c  |  58 ++
 src/storage/storage_driver.h  |   3 +
 10 files changed, 277 insertions(+), 47 deletions(-)

-- 
2.0.0

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


[libvirt] [PATCH 3/8] security: DAC: Remove superfluous link resolution

2014-07-10 Thread Peter Krempa
When restoring security labels in the dac driver the code would resolve
the file path and use the resolved one to be chown-ed. The setting code
doesn't do that. Remove the unnecessary code.
---
 src/security/security_dac.c | 19 +--
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 26cd615..6821d37 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -264,27 +264,10 @@ virSecurityDACSetOwnership(const char *path, uid_t uid, 
gid_t gid)
 static int
 virSecurityDACRestoreSecurityFileLabel(const char *path)
 {
-struct stat buf;
-int rc = -1;
-char *newpath = NULL;
-
 VIR_INFO(Restoring DAC user and group on '%s', path);

-if (virFileResolveLink(path, newpath)  0) {
-virReportSystemError(errno,
- _(cannot resolve symlink %s), path);
-goto err;
-}
-
-if (stat(newpath, buf) != 0)
-goto err;
-
 /* XXX record previous ownership */
-rc = virSecurityDACSetOwnership(newpath, 0, 0);
-
- err:
-VIR_FREE(newpath);
-return rc;
+return virSecurityDACSetOwnership(path, 0, 0);
 }


-- 
2.0.0

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


[libvirt] [PATCH 5/8] security: DAC: Plumb usage of chown callback

2014-07-10 Thread Peter Krempa
Use the callback to set disk and storage image labels by modifying the
existing functions and adding wrappers to avoid refactoring a lot of the
code.
---
 src/security/security_dac.c | 89 +++--
 1 file changed, 69 insertions(+), 20 deletions(-)

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index eafd714..0ad024c 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -230,23 +230,54 @@ virSecurityDACPreFork(virSecurityManagerPtr mgr)
 }

 static int
-virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid)
+virSecurityDACSetOwnershipInternal(virSecurityDACDataPtr priv,
+   virStorageSourcePtr src,
+   const char *path,
+   uid_t uid,
+   gid_t gid)
 {
+int rc;
+int chown_errno;
+
 VIR_INFO(Setting DAC user and group on '%s' to '%ld:%ld',
- path, (long) uid, (long) gid);
+ NULLSTR(src ? src-path : path), (long) uid, (long) gid);
+
+if (priv  src  priv-chownCallback) {
+rc = priv-chownCallback(src, uid, gid);
+
+/* on -2 returned an error was already reported */
+if (rc == -2)
+return -1;

-if (chown(path, uid, gid)  0) {
+/* on -1 only errno was set */
+chown_errno = errno;
+} else {
 struct stat sb;
-int chown_errno = errno;

-if (stat(path, sb) = 0) {
+if (!path) {
+if (!src || !src-path)
+return 0;
+
+if (!virStorageSourceIsLocalStorage(src))
+return 0;
+
+path = src-path;
+}
+
+rc = chown(path, uid, gid);
+chown_errno = errno;
+
+if (rc  0 
+stat(path, sb) = 0) {
 if (sb.st_uid == uid 
 sb.st_gid == gid) {
 /* It's alright, there's nothing to change anyway. */
 return 0;
 }
 }
+}

+if (rc  0) {
 if (chown_errno == EOPNOTSUPP || chown_errno == EINVAL) {
 VIR_INFO(Setting user and group to '%ld:%ld' on '%s' not 
  supported by filesystem,
@@ -270,13 +301,31 @@ virSecurityDACSetOwnership(const char *path, uid_t uid, 
gid_t gid)
 return 0;
 }

+
 static int
-virSecurityDACRestoreSecurityFileLabel(const char *path)
+virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid)
+{
+return virSecurityDACSetOwnershipInternal(NULL, NULL, path, uid, gid);
+}
+
+
+static int
+virSecurityDACRestoreSecurityFileLabelInternal(virSecurityDACDataPtr priv,
+   virStorageSourcePtr src,
+   const char *path)
 {
-VIR_INFO(Restoring DAC user and group on '%s', path);
+VIR_INFO(Restoring DAC user and group on '%s',
+ NULLSTR(src ? src-path : path));

 /* XXX record previous ownership */
-return virSecurityDACSetOwnership(path, 0, 0);
+return virSecurityDACSetOwnershipInternal(priv, src, path, 0, 0);
+}
+
+
+static int
+virSecurityDACRestoreSecurityFileLabel(const char *path)
+{
+return virSecurityDACRestoreSecurityFileLabelInternal(NULL, NULL, path);
 }


@@ -294,10 +343,6 @@ virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr 
mgr,
 if (!priv-dynamicOwnership)
 return 0;

-/* XXX: Add support for gluster DAC permissions */
-if (!src-path || !virStorageSourceIsLocalStorage(src))
-return 0;
-
 secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
 if (secdef  secdef-norelabel)
 return 0;
@@ -315,7 +360,7 @@ virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr 
mgr,
 return -1;
 }

-return virSecurityDACSetOwnership(src-path, user, group);
+return virSecurityDACSetOwnershipInternal(priv, src, NULL, user, group);
 }


@@ -349,9 +394,6 @@ 
virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr,
 if (!priv-dynamicOwnership)
 return 0;

-if (!src-path || !virStorageSourceIsLocalStorage(src))
-return 0;
-
 /* Don't restore labels on readoly/shared disks, because other VMs may
  * still be accessing these. Alternatively we could iterate over all
  * running domains and try to figure out if it is in use, but this would
@@ -373,9 +415,16 @@ 
virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr,
  * ownership, because that kills access on the destination host which is
  * sub-optimal for the guest VM's I/O attempts :-) */
 if (migrated) {
-int rc = virFileIsSharedFS(src-path);
-if (rc  0)
-return -1;
+int rc = 1;
+
+if (virStorageSourceIsLocalStorage(src)) {
+if (!src-path)
+return 0;
+
+if ((rc = virFileIsSharedFS(src-path))  0)
+return -1;
+

[libvirt] [PATCH 1/8] storage: Implement storage driver helper to chown disk images

2014-07-10 Thread Peter Krempa
Gluster storage works on a similar principle to NFS where it takes the
uid and gid of the actual process and uses it to access the storage
volume on the remote server. This introduces a need to chown storage
files on gluster via native API.
---
 src/storage/storage_backend.h |  6 ++
 src/storage/storage_backend_fs.c  | 12 
 src/storage/storage_backend_gluster.c | 12 
 src/storage/storage_driver.c  | 28 
 src/storage/storage_driver.h  |  1 +
 5 files changed, 59 insertions(+)

diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 76c1afa..1a80aef 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -202,6 +202,11 @@ typedef int
 (*virStorageFileBackendAccess)(virStorageSourcePtr src,
int mode);

+typedef int
+(*virStorageFileBackendChown)(virStorageSourcePtr src,
+  uid_t uid,
+  gid_t gid);
+
 virStorageFileBackendPtr virStorageFileBackendForType(int type, int protocol);
 virStorageFileBackendPtr virStorageFileBackendForTypeInternal(int type,
   int protocol,
@@ -227,6 +232,7 @@ struct _virStorageFileBackend {
 virStorageFileBackendUnlink storageFileUnlink;
 virStorageFileBackendStat   storageFileStat;
 virStorageFileBackendAccess storageFileAccess;
+virStorageFileBackendChown  storageFileChown;
 };

 #endif /* __VIR_STORAGE_BACKEND_H__ */
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index c93fc1e..693ebd3 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1435,6 +1435,15 @@ virStorageFileBackendFileAccess(virStorageSourcePtr src,
 }


+static int
+virStorageFileBackendFileChown(virStorageSourcePtr src,
+   uid_t uid,
+   gid_t gid)
+{
+return chown(src-path, uid, gid);
+}
+
+
 virStorageFileBackend virStorageFileBackendFile = {
 .type = VIR_STORAGE_TYPE_FILE,

@@ -1445,6 +1454,7 @@ virStorageFileBackend virStorageFileBackendFile = {
 .storageFileStat = virStorageFileBackendFileStat,
 .storageFileReadHeader = virStorageFileBackendFileReadHeader,
 .storageFileAccess = virStorageFileBackendFileAccess,
+.storageFileChown = virStorageFileBackendFileChown,

 .storageFileGetUniqueIdentifier = 
virStorageFileBackendFileGetUniqueIdentifier,
 };
@@ -1459,6 +1469,7 @@ virStorageFileBackend virStorageFileBackendBlock = {
 .storageFileStat = virStorageFileBackendFileStat,
 .storageFileReadHeader = virStorageFileBackendFileReadHeader,
 .storageFileAccess = virStorageFileBackendFileAccess,
+.storageFileChown = virStorageFileBackendFileChown,

 .storageFileGetUniqueIdentifier = 
virStorageFileBackendFileGetUniqueIdentifier,
 };
@@ -1471,6 +1482,7 @@ virStorageFileBackend virStorageFileBackendDir = {
 .backendDeinit = virStorageFileBackendFileDeinit,

 .storageFileAccess = virStorageFileBackendFileAccess,
+.storageFileChown = virStorageFileBackendFileChown,

 .storageFileGetUniqueIdentifier = 
virStorageFileBackendFileGetUniqueIdentifier,
 };
diff --git a/src/storage/storage_backend_gluster.c 
b/src/storage/storage_backend_gluster.c
index 76d2461..edb0511 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -754,6 +754,17 @@ 
virStorageFileBackendGlusterGetUniqueIdentifier(virStorageSourcePtr src)
 }


+static int
+virStorageFileBackendGlusterChown(virStorageSourcePtr src,
+  uid_t uid,
+  gid_t gid)
+{
+virStorageFileBackendGlusterPrivPtr priv = src-drv-priv;
+
+return glfs_chown(priv-vol, src-path, uid, gid);
+}
+
+
 virStorageFileBackend virStorageFileBackendGluster = {
 .type = VIR_STORAGE_TYPE_NETWORK,
 .protocol = VIR_STORAGE_NET_PROTOCOL_GLUSTER,
@@ -765,6 +776,7 @@ virStorageFileBackend virStorageFileBackendGluster = {
 .storageFileStat = virStorageFileBackendGlusterStat,
 .storageFileReadHeader = virStorageFileBackendGlusterReadHeader,
 .storageFileAccess = virStorageFileBackendGlusterAccess,
+.storageFileChown = virStorageFileBackendGlusterChown,

 .storageFileGetUniqueIdentifier = 
virStorageFileBackendGlusterGetUniqueIdentifier,

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index ae86c69..928fadf 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2823,6 +2823,34 @@ virStorageFileAccess(virStorageSourcePtr src,
 }


+/**
+ * virStorageFileChown: Change owner of a storage file
+ *
+ * @src: storage file to change owner of
+ * @uid: new owner id
+ * @gid: new group id
+ *
+ * Returns 0 on success, -1 on error and sets errno. No libvirt
+ * error is reported. Returns -2 if the operation isn't supported
+ * by libvirt storage 

[libvirt] [PATCH 0/3] storage: Fix operations working with local storage only

2014-07-10 Thread Peter Krempa
Peter Krempa (3):
  conf: storage: Add helper to determine whether storage vol is local
  storage: wipe: Don't attempt to wipe remote storage
  storage: Split out volume upload/download as separate backend function

 src/conf/storage_conf.c   | 19 +
 src/conf/storage_conf.h   |  2 ++
 src/libvirt_private.syms  |  1 +
 src/storage/storage_backend.c | 31 
 src/storage/storage_backend.h | 30 
 src/storage/storage_backend_disk.c|  2 ++
 src/storage/storage_backend_fs.c  |  7 +
 src/storage/storage_backend_iscsi.c   |  2 ++
 src/storage/storage_backend_logical.c |  2 ++
 src/storage/storage_backend_mpath.c   |  2 ++
 src/storage/storage_backend_scsi.c|  2 ++
 src/storage/storage_driver.c  | 53 ++-
 12 files changed, 121 insertions(+), 32 deletions(-)

-- 
2.0.0

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


[libvirt] [PATCH 2/3] storage: wipe: Don't attempt to wipe remote storage

2014-07-10 Thread Peter Krempa
Volume wiping uses external tool to wipe the storage. This will not work
with purely remote storage as native gluster or RBD. Add a check to
report better error when wiping a remote volume.

Before:
 $ virsh vol-wipe --pool glusterpool qcow3
 error: Failed to wipe vol qcow3
 error: Failed to open storage volume with path 
'gluster://gluster-node-1/gv0/qcow3': No such file or directory

After:
 $ virsh vol-wipe --pool glusterpool qcow3
 error: Failed to wipe vol qcow3
 error: Operation not supported: Volume wiping is not supported on non-local 
storage
---
 src/storage/storage_driver.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index ae86c69..7303dbf 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2226,6 +2226,12 @@ storageVolWipeInternal(virStorageVolDefPtr def,
 VIR_DEBUG(Wiping volume with path '%s' and algorithm %u,
   def-target.path, algorithm);

+if (!virStorageVolIsLocalStorage(def)) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+   _(Volume wiping is not supported on non-local 
storage));
+goto cleanup;
+}
+
 fd = open(def-target.path, O_RDWR);
 if (fd == -1) {
 virReportSystemError(errno,
-- 
2.0.0

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


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

2014-07-10 Thread Peter Krempa
For non-local storage drivers we can't expect to use the FDStream
backend for up/downloading volumes. Split the code into a separate
backend function so that we can add protocol specific code later.
---
 src/storage/storage_backend.c | 31 +++
 src/storage/storage_backend.h | 30 ++
 src/storage/storage_backend_disk.c|  2 ++
 src/storage/storage_backend_fs.c  |  7 ++
 src/storage/storage_backend_iscsi.c   |  2 ++
 src/storage/storage_backend_logical.c |  2 ++
 src/storage/storage_backend_mpath.c   |  2 ++
 src/storage/storage_backend_scsi.c|  2 ++
 src/storage/storage_driver.c  | 47 +++
 9 files changed, 93 insertions(+), 32 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index e83a108..7b17ca4 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -56,6 +56,7 @@
 #include stat-time.h
 #include virstring.h
 #include virxml.h
+#include fdstream.h

 #if WITH_STORAGE_LVM
 # include storage_backend_logical.h
@@ -1669,6 +1670,36 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool,
 return stablepath;
 }

+int
+virStorageBackendVolUploadLocal(virConnectPtr conn ATTRIBUTE_UNUSED,
+virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
+virStorageVolDefPtr vol,
+virStreamPtr stream,
+unsigned long long offset,
+unsigned long long len,
+unsigned int flags)
+{
+virCheckFlags(0, -1);
+
+/* Not using O_CREAT because the file is required to already exist at
+ * this point */
+return virFDStreamOpenFile(stream, vol-target.path, offset, len, 
O_WRONLY);
+}
+
+int
+virStorageBackendVolDownloadLocal(virConnectPtr conn ATTRIBUTE_UNUSED,
+  virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
+  virStorageVolDefPtr vol,
+  virStreamPtr stream,
+  unsigned long long offset,
+  unsigned long long len,
+  unsigned int flags)
+{
+virCheckFlags(0, -1);
+
+return virFDStreamOpenFile(stream, vol-target.path, offset, len, 
O_RDONLY);
+}
+
 #ifdef GLUSTER_CLI
 int
 virStorageBackendFindGlusterPoolSources(const char *host,
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 76c1afa..4d7d4b0 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -73,6 +73,20 @@ typedef int (*virStorageBackendVolumeResize)(virConnectPtr 
conn,
  virStorageVolDefPtr vol,
  unsigned long long capacity,
  unsigned int flags);
+typedef int (*virStorageBackendVolumeDownload)(virConnectPtr conn,
+   virStoragePoolObjPtr obj,
+   virStorageVolDefPtr vol,
+   virStreamPtr stream,
+   unsigned long long offset,
+   unsigned long long length,
+   unsigned int flags);
+typedef int (*virStorageBackendVolumeUpload)(virConnectPtr conn,
+ virStoragePoolObjPtr obj,
+ virStorageVolDefPtr vol,
+ virStreamPtr stream,
+ unsigned long long offset,
+ unsigned long long len,
+ unsigned int flags);

 /* File creation/cloning functions used for cloning between backends */
 int virStorageBackendCreateRaw(virConnectPtr conn,
@@ -91,6 +105,20 @@ int virStorageBackendFindGlusterPoolSources(const char 
*host,
 int pooltype,
 virStoragePoolSourceListPtr list);

+int virStorageBackendVolUploadLocal(virConnectPtr conn,
+virStoragePoolObjPtr pool,
+virStorageVolDefPtr vol,
+virStreamPtr stream,
+unsigned long long offset,
+unsigned long long len,
+unsigned int flags);
+int virStorageBackendVolDownloadLocal(virConnectPtr conn,
+  virStoragePoolObjPtr pool,
+  virStorageVolDefPtr vol,
+  virStreamPtr stream,
+ 

[libvirt] [PATCH 1/3] conf: storage: Add helper to determine whether storage vol is local

2014-07-10 Thread Peter Krempa
Add a simple helper that returns true if a storage vol definition points
to a volume accessible as a file in the host.
---
 src/conf/storage_conf.c  | 19 +++
 src/conf/storage_conf.h  |  2 ++
 src/libvirt_private.syms |  1 +
 3 files changed, 22 insertions(+)

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 9ac5975..55e44b2 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -335,6 +335,25 @@ virStorageVolDefFree(virStorageVolDefPtr def)
 VIR_FREE(def);
 }

+/* Returns true if storage volume can be accessed as a file in the host */
+bool
+virStorageVolIsLocalStorage(virStorageVolDefPtr vol)
+{
+switch ((virStorageVolType) vol-type) {
+case VIR_STORAGE_VOL_FILE:
+case VIR_STORAGE_VOL_BLOCK:
+case VIR_STORAGE_VOL_DIR:
+return true;
+
+case VIR_STORAGE_VOL_NETWORK:
+case VIR_STORAGE_VOL_NETDIR:
+case VIR_STORAGE_VOL_LAST:
+return false;
+}
+
+return false;
+}
+
 static void
 virStoragePoolSourceAdapterClear(virStoragePoolSourceAdapter adapter)
 {
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 47f769b..175c3b7 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -328,6 +328,8 @@ virStorageVolDefPtr
 virStorageVolDefFindByName(virStoragePoolObjPtr pool,
const char *name);

+bool virStorageVolIsLocalStorage(virStorageVolDefPtr vol);
+
 void virStoragePoolObjClearVols(virStoragePoolObjPtr pool);

 virStoragePoolDefPtr virStoragePoolDefParseString(const char *xml);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e59ea4c..ccb1de7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -764,6 +764,7 @@ virStorageVolDefFree;
 virStorageVolDefParseFile;
 virStorageVolDefParseNode;
 virStorageVolDefParseString;
+virStorageVolIsLocalStorage;
 virStorageVolTypeFromString;
 virStorageVolTypeToString;

-- 
2.0.0

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


Re: [libvirt] [PATCHv5 0/4] Outline of rewriting vbox Driver

2014-07-10 Thread Michal Privoznik

On 02.07.2014 18:53, Taowei wrote:

Fix some problems in last patch. Includes:
  * Re-manage the patches in a better order, move some symbol definitions into 
the proper patch.
  * Make the API in vboxUniformedAPI hierarchical. So when more apis introduced 
into the
uniformed api, it wouldn't go to a mess.
  * Rewrite the vboxDomainSave in a better way.

Taowei (4):
   Define vboxUniformedAPI
   Implement vboxInitialize and vboxDomainSave
   Implement uniformed api for each vbox version
   Install vboxUniformedAPI

  po/POTFILES.in|1 +
  src/Makefile.am   |4 +-
  src/vbox/vbox_common.c|  183 +++
  src/vbox/vbox_common.h|  151 
  src/vbox/vbox_driver.c|   22 +-
  src/vbox/vbox_tmpl.c  |  504 -
  src/vbox/vbox_uniformed_api.h |  211 +
  7 files changed, 868 insertions(+), 208 deletions(-)
  create mode 100644 src/vbox/vbox_common.c
  create mode 100644 src/vbox/vbox_common.h
  create mode 100644 src/vbox/vbox_uniformed_api.h



Okay, all the patches works for me. I think you can start rewriting more 
APIs.


Michal

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


Re: [libvirt] [PATCH v2 1/4] virSecurityLabelDef: substitute 'norelabel' with 'relabel'

2014-07-10 Thread Ján Tomko
On 07/10/2014 04:04 PM, Michal Privoznik wrote:
 This negation in names of boolean variables is driving me insane. The
 code is much more readable if we drop the 'no-' prefix. Well, at least
 for me.
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  src/conf/domain_conf.c   | 20 ++--
  src/security/security_apparmor.c | 10 +-
  src/security/security_dac.c  | 14 +++---
  src/security/security_manager.c  |  2 +-
  src/security/security_selinux.c  | 24 ++--
  src/util/virseclabel.h   |  2 +-
  6 files changed, 34 insertions(+), 38 deletions(-)
 

 diff --git a/src/security/security_manager.c b/src/security/security_manager.c
 index 16bec5c..8a45e04 100644
 --- a/src/security/security_manager.c
 +++ b/src/security/security_manager.c
 @@ -616,7 +616,7 @@ virSecurityManagerGenLabel(virSecurityManagerPtr mgr,
  seclabel-type = VIR_DOMAIN_SECLABEL_DYNAMIC;

seclabel-relabel = true;
is needed here now, since the code was relying on norelabel being false by
default to enable relabeling (and I agree with your comment about readability
now :))

The new default also affects the other caller of virSecurityLabelDefNew:
In qemuProcessAttach where we generate a new label:

if (seclabeldef == NULL) {
if (!(seclabeldef = virSecurityLabelDefNew(model)))
goto error;
seclabelgen = true;
}

I'd set relabel to true here, to make this commit a no-op.


  } else {
  seclabel-type = VIR_DOMAIN_SECLABEL_NONE;
 -seclabel-norelabel = true;
 +seclabel-relabel = false;
  }
  }
  

 diff --git a/src/util/virseclabel.h b/src/util/virseclabel.h
 index b90d212..8d671fd 100644
 --- a/src/util/virseclabel.h
 +++ b/src/util/virseclabel.h
 @@ -40,7 +40,7 @@ struct _virSecurityLabelDef {
  char *imagelabel;   /* security image label string */
  char *baselabel;/* base name of label string */
  int type;   /* virDomainSeclabelType */
 -bool norelabel;
 +bool relabel;   /* should try labeling attempts? */

I can't parse that. How about whether we relabel files, or just leaving it
without a comment?

ACK with the two callers of virSecurityLabelDefNew fixed.

Jan



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

Re: [libvirt] [PATCH v2 2/4] virSecurityDeviceLabelDef: substitute 'norelabel' with 'relabel'

2014-07-10 Thread Ján Tomko
On 07/10/2014 04:04 PM, Michal Privoznik wrote:
 Similarly to the previous commit, boolean variables should not start
 with 'no-' prefix.
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  src/conf/domain_conf.c  | 12 ++--
  src/security/security_dac.c |  8 
  src/security/security_selinux.c | 10 +-
  src/util/virseclabel.c  |  2 +-
  src/util/virseclabel.h  |  2 +-
  5 files changed, 17 insertions(+), 17 deletions(-)
 

 diff --git a/src/util/virseclabel.h b/src/util/virseclabel.h
 index 8d671fd..7eebd60 100644
 --- a/src/util/virseclabel.h
 +++ b/src/util/virseclabel.h
 @@ -51,7 +51,7 @@ typedef virSecurityDeviceLabelDef 
 *virSecurityDeviceLabelDefPtr;
  struct _virSecurityDeviceLabelDef {
  char *model;
  char *label;/* image label string */
 -bool norelabel; /* true to skip label attempts */
 +bool relabel;   /* should try labeling attempts? */

Same issue as in the first patch.

  bool labelskip; /* live-only; true if skipping failed label attempt 
 */
  };

ACK

Jan



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

Re: [libvirt] [PATCH 4/4] vhost-user support: lxc,xenxs,uml

2014-07-10 Thread Michal Privoznik

On 02.07.2014 15:20, Michele Paolino wrote:

vhost-user is a qemu feature. Initial support for lxc,uml and
xenxs added.

Signed-off-by: Michele Paolino m.paol...@virtualopensystems.com
---
  src/lxc/lxc_process.c | 1 +
  src/uml/uml_conf.c| 5 +
  src/xenxs/xen_sxpr.c  | 1 +
  3 files changed, 7 insertions(+)


Right. Well, here at libvirt we require the sources to be able to 
compile after each commit. So this needs to go into 1/4.


Michal

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


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

2014-07-10 Thread Michal Privoznik

On 02.07.2014 15:20, Michele Paolino wrote:

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

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



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



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

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

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


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



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


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



+  /choice
+/attribute
+attribute name=path
+  ref name=absFilePath/
+/attribute
+attribute name=mode
+  choice
+valueserver/value
+valueclient/value
+  /choice
+/attribute
+empty/
+  /element
+/optional
+ref name=interface-options/
+  /interleave
+/group
+group
+  attribute name=type
  valuenetwork/value
/attribute
interleave
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args 
b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args
new file mode 100644
index 000..cc66ec3
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args
@@ -0,0 +1,7 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/qemu -S -M \
+pc -m 214 -smp 1 -nographic -nodefaults -monitor 
unix:/tmp/test-monitor,server,nowait \
+-no-acpi -boot c -usb 

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

2014-07-10 Thread Michal Privoznik
On 02.07.2014 15:20, Michele Paolino wrote:
 vhost-user is added as a virDomainChrSourceDefPtr variable in the
 virtual network interface configuration structure.
 This patch adds support to parse vhost-user element. The XML file
 looks like:
 
 interface type='vhostuser'
  source type='unix' path='/tmp/vhost.sock' mode='server'/
  mac address='52:54:00:3b:83:1a'/
  model type='virtio'/
 /interface
 
 Signed-off-by: Michele Paolino m.paol...@virtualopensystems.com
 ---
   src/conf/domain_conf.c | 81 
 ++
   src/conf/domain_conf.h | 10 +--
   2 files changed, 89 insertions(+), 2 deletions(-)

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

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

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

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

Ouch, in the code I've commented above, you allow vhostuser_type to be a NULL, 
so this needs to be STREQ_NULLABLE().

 +
 +(def-data.vhostuser)-type = VIR_DOMAIN_CHR_TYPE_UNIX;

I haven't seen such braces usage since DV stopped writing patches :)

 +
 +if (vhostuser_path == NULL) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(No source 'path' attribute 
 +   specified with interface 
 +   type='vhostuser'/));
 +goto error;
 +}
 +
 +(def-data.vhostuser)-data.nix.path = vhostuser_path;
 +
 +if (vhostuser_mode == NULL) {
 +

Re: [libvirt] [PATCH v2 3/4] conf: Always format seclabel's model

2014-07-10 Thread Ján Tomko
On 07/10/2014 04:04 PM, Michal Privoznik wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1113860
 
 We've always done that. Well, until 990e46c45. Point is, if we don't
 format model, we may lose a domain on libvirtd restart. If the
 seclabel is implicit however, we should skip it's formatting.
 

It seems we only generate a seclabel with model=none with type=none.
Specifying other types (or the relabel attribute) with model=none doesn't
make much sense.

Could we instead error out on other types than none when model is none, or
(if we can't) silently correct it to type none?

Also, formatting model=none in the status XML shouldn't be needed after that
since we only generate it with type=none and we would reject other types.

Jan



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

Re: [libvirt] [PATCH 2/4] vhost-user support: qemu command-line

2014-07-10 Thread Michal Privoznik

On 02.07.2014 15:20, Michele Paolino wrote:

Build the qemu-command line for vhost-user.

Signed-off-by: Michele Paolino m.paol...@virtualopensystems.com
---
  src/qemu/qemu_command.c | 58 +
  1 file changed, 58 insertions(+)


This is the patch where .args and qemuxml2argvtest.c modifications would 
have been done if you go with my suggestion in 1/4.




diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index ab8cec5..c1cf001 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6930,6 +6930,61 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg,
  }

  static int
+qemuBuildVhostuserCommandLine(virCommandPtr cmd,
+  virDomainDefPtr def,
+  virDomainNetDefPtr net,
+  virQEMUCapsPtr qemuCaps)
+{
+virBuffer chardev_buf = VIR_BUFFER_INITIALIZER;
+virBuffer netdev_buf = VIR_BUFFER_INITIALIZER;
+
+char* nic = NULL;
+
+if (!qemuDomainSupportsNetdev(def, qemuCaps, net)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   %s, _(Netdev support unavailable));
+goto error;
+}
+
+if ((net-data.vhostuser)-type == VIR_DOMAIN_CHR_TYPE_UNIX) {
+virBufferAsprintf(chardev_buf, socket,id=char%s,path=%s%s,
+  net-info.alias, 
(net-data.vhostuser)-data.nix.path,
+  (net-data.vhostuser)-data.nix.listen ? ,server : 
);
+}
+
+virBufferAsprintf(netdev_buf, type=vhost-user,id=host%s,chardev=char%s,
+  net-info.alias, net-info.alias);
+
+if (virBufferError(chardev_buf) || virBufferError(netdev_buf)) {
+virReportOOMError();
+goto error;
+}
+
+virCommandAddArgList(cmd, -chardev, 
virBufferContentAndReset(chardev_buf),
+ NULL);
+virCommandAddArgList(cmd, -netdev, virBufferContentAndReset(netdev_buf),
+ NULL);


For some reason we tend to store the return value of 
virBufferContentAndReset() into a local variable which is then passed to 
virCommandAddArgList(). Maybe for better debugging options.



+
+if (!(nic = qemuBuildNicDevStr(def, net, -1, 0, 0, qemuCaps))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   %s, _(Error generating NIC -device string));
+goto error;
+}
+
+virCommandAddArgList(cmd, -device, nic, NULL);
+VIR_FREE(nic);
+
+return 0;
+
+ error:
+virBufferFreeAndReset(chardev_buf);
+virBufferFreeAndReset(netdev_buf);
+VIR_FREE(nic);
+
+return -1;
+}
+
+static int
  qemuBuildInterfaceCommandLine(virCommandPtr cmd,
virQEMUDriverPtr driver,
virConnectPtr conn,
@@ -6952,6 +7007,9 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
  int actualType = virDomainNetGetActualType(net);
  size_t i;

+if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER)
+return qemuBuildVhostuserCommandLine(cmd, def, net, qemuCaps);
+
  if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
  /* NET_TYPE_HOSTDEV devices are really hostdev devices, so
   * their commandlines are constructed with other hostdevs.



Michal

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


Re: [libvirt] [PATCH v2 4/4] conf: Don't allow multiple seclabels for same model

2014-07-10 Thread Ján Tomko
On 07/10/2014 04:04 PM, Michal Privoznik wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1066894
 
 With current code it's possible to have for instance:
 
 virsh dumpxml mydomain | grep seclabel
   seclabel type='dynamic' model='selinux' relabel='yes'/
   seclabel type='dynamic' model='selinux' relabel='yes'/
   seclabel type='dynamic' model='selinux' relabel='yes'/
   seclabel type='dynamic' model='selinux' relabel='yes'/
   seclabel type='dynamic' model='selinux' relabel='yes'/
 
 what doesn't make any sense. We should reject the XML in the config

s/what/which/

 parsing phase.
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  src/conf/domain_conf.c | 18 --
  .../qemuxml2argv-seclabel-multiple.xml | 40 
 ++
  tests/qemuxml2argvtest.c   |  1 +
  3 files changed, 56 insertions(+), 3 deletions(-)
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-multiple.xml
 

 @@ -4689,10 +4689,22 @@ virSecurityLabelDefsParseXML(virDomainDefPtr def,
  
  /* Parse each seclabel tag */
  for (i = 0; i  n; i++) {
 +virSecurityLabelDefPtr seclabel;
 +
  ctxt-node = list[i];
 -def-seclabels[i] = virSecurityLabelDefParseXML(ctxt, flags);
 -if (def-seclabels[i] == NULL)
 +if (!(seclabel = virSecurityLabelDefParseXML(ctxt, flags)))
  goto error;
 +
 +for (j = 0; j  i; j++) {
 +if (STREQ_NULLABLE(seclabel-model, def-seclabels[j]-model)) {
 +virReportError(VIR_ERR_XML_DETAIL,
 +   _(seclablel for model %s is already 
 provided),
 +   seclabel-model);

virSecurityLabelDefFree(seclabel);

 +goto error;
 +}
 +}
 +
 +def-seclabels[i] = seclabel;
  }
  def-nseclabels = n;
  ctxt-node = saved_node;

ACK with the leak fixed.

Jan



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

Re: [libvirt] [PATCH 0/3] storage: Fix operations working with local storage only

2014-07-10 Thread Ján Tomko
On 07/10/2014 04:30 PM, Peter Krempa wrote:
 Peter Krempa (3):
   conf: storage: Add helper to determine whether storage vol is local
   storage: wipe: Don't attempt to wipe remote storage
   storage: Split out volume upload/download as separate backend function
 
  src/conf/storage_conf.c   | 19 +
  src/conf/storage_conf.h   |  2 ++
  src/libvirt_private.syms  |  1 +
  src/storage/storage_backend.c | 31 
  src/storage/storage_backend.h | 30 
  src/storage/storage_backend_disk.c|  2 ++
  src/storage/storage_backend_fs.c  |  7 +
  src/storage/storage_backend_iscsi.c   |  2 ++
  src/storage/storage_backend_logical.c |  2 ++
  src/storage/storage_backend_mpath.c   |  2 ++
  src/storage/storage_backend_scsi.c|  2 ++
  src/storage/storage_driver.c  | 53 
 ++-
  12 files changed, 121 insertions(+), 32 deletions(-)
 

ACK series

Jan



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

Re: [libvirt] [PREPOST 01/17] src/xenxs:Introduce function

2014-07-10 Thread Eric Blake
On 07/10/2014 07:42 AM, David Kiarie wrote:
 From: Kiarie Kahurani davidkiar...@gmail.com
 
 Introduce function xenParseXMGeneral(virConfPtr conf, ...);
 This function parses the xm general options such as uuid and name

[meta-comment]:

When posting a series, it's useful to post a cover letter (0/17) to
which all patches reply to, rather than having 2-17 reply to 1/17 with
no lead-in on the series as a whole.

 
 signed-off-by: David Kiarie davidkiar...@gmail.com
 ---
  src/xenxs/xen_xm.c | 65 
 +++---
  1 file changed, 37 insertions(+), 28 deletions(-)
 
 diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
 index 25a042d..1953a85 100644
 --- a/src/xenxs/xen_xm.c
 +++ b/src/xenxs/xen_xm.c
 @@ -253,44 +253,25 @@ xenXMConfigGetUUID(virConfPtr conf, const char *name, 
 unsigned char *uuid)
   * Turn a config record into a lump of XML describing the
   * domain, suitable for later feeding for virDomainCreateXML
   */
 -virDomainDefPtr
 -xenParseXM(virConfPtr conf, int xendConfigVersion,
 -   virCapsPtr caps)
 +static int xenParseXMGeneral(virConfPtr conf, virDomainDefPtr def,
 +  virCapsPtr caps)

Indentation is off; per our style, this should be:

static int
xenParseXMGeneral(virConfPtr conf, virDomainDefPtr def,
  virCapsPtr caps)


 +}
 +virDomainDefPtr

2 blank lines between functions.

 +xenParseXM(virConfPtr conf, int xendConfigVersion,
 +   virCapsPtr caps)

Indentation is off; the second line should line up to the character
after the ( of the first line.

Otherwise, this looks like it is just a refactoring of splitting out a
helper function for later reuse.

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



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

Re: [libvirt] [PATCH v2 1/4] virSecurityLabelDef: substitute 'norelabel' with 'relabel'

2014-07-10 Thread Eric Blake
On 07/10/2014 09:02 AM, Ján Tomko wrote:
 On 07/10/2014 04:04 PM, Michal Privoznik wrote:
 This negation in names of boolean variables is driving me insane. The
 code is much more readable if we drop the 'no-' prefix. Well, at least
 for me.

 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---

 +++ b/src/security/security_manager.c
 @@ -616,7 +616,7 @@ virSecurityManagerGenLabel(virSecurityManagerPtr mgr,
  seclabel-type = VIR_DOMAIN_SECLABEL_DYNAMIC;
 
 seclabel-relabel = true;
 is needed here now, since the code was relying on norelabel being false by
 default to enable relabeling (and I agree with your comment about readability
 now :))
 
 The new default also affects the other caller of virSecurityLabelDefNew:
 In qemuProcessAttach where we generate a new label:
 
 if (seclabeldef == NULL) {
 if (!(seclabeldef = virSecurityLabelDefNew(model)))
 goto error;
 seclabelgen = true;
 }
 
 I'd set relabel to true here, to make this commit a no-op.

Actually, that would argue that virSecurityLabelDefNew() (and any other
allocation function) should set the relabel=true as PART of the
allocation.  Any time that we rely on a default state, if the state is
all 0 then VIR_ALLOC is okay, and if the default state requires
non-zero, then all clients should use an allocation function and the
allocation function should take care of the default.

For example, see the commit pair bc3f5f1 (where I ensured that all
clients allocate via a common function) and c123ef7 (where I used that
common allocation function to initialize a non-zero member, rather than
having to touch each caller).

 +++ b/src/util/virseclabel.h
 @@ -40,7 +40,7 @@ struct _virSecurityLabelDef {
  char *imagelabel;   /* security image label string */
  char *baselabel;/* base name of label string */
  int type;   /* virDomainSeclabelType */
 -bool norelabel;
 +bool relabel;   /* should try labeling attempts? */
 
 I can't parse that. How about whether we relabel files, or just leaving it
 without a comment?

or even true (default) for allowing relabels

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



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

Re: [libvirt] [PATCH 2/3] storage: wipe: Don't attempt to wipe remote storage

2014-07-10 Thread Eric Blake
On 07/10/2014 08:30 AM, Peter Krempa wrote:
 Volume wiping uses external tool to wipe the storage. This will not work
 with purely remote storage as native gluster or RBD. Add a check to
 report better error when wiping a remote volume.
 
 Before:
  $ virsh vol-wipe --pool glusterpool qcow3
  error: Failed to wipe vol qcow3
  error: Failed to open storage volume with path 
 'gluster://gluster-node-1/gv0/qcow3': No such file or directory
 
 After:
  $ virsh vol-wipe --pool glusterpool qcow3
  error: Failed to wipe vol qcow3
  error: Operation not supported: Volume wiping is not supported on non-local 
 storage

Technically, remote wiping MAY be possible (in fact, it may even be MORE
efficient than calling out to external storage - doesn't gluster provide
some APIs to request background file wiping?).  But that's where we'd
need per-pool callbacks for handling the request (local storage uses the
existing code, remote storage supplies new callbacks to hook into the
proper wipe protocol for that storage).

I'd feel a bit better if we refactored this function into delegating the
wipe to a _backend.c file callback, and failing when the callback is not
yet defined, rather than open-coding it to be local-only.

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



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

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

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

Without even reading this patch yet, this is more the approach I had in
mind for 2/3.

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



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

Re: [libvirt] Bug: iohelper drops I/O error messages

2014-07-10 Thread Jason J. Herne

On 06/24/2014 03:21 PM, Eric Blake wrote:

On 06/24/2014 08:07 AM, Jason J. Herne wrote:

During a recent managed save operation I received the following error
message:

  error: operation failed: domain save job: unexpectedly failed.

It turns out that I had run out of disk space. After a brief
investigation I
discovered that libvirt_iohelper is exec'ed and is used to handle all
I/O during
a (Qemu) managed save operation. While iohelper appears to be set up to log
error conditions when they occur, for some reason the logging is getting
lost.
I'm hoping someone can help figure out why these errors are getting
lost.


What version of libvirt are you using, in case it is something that has
been improved in the meantime?



I tried this again today with a recent version pulled from git. The 
following is the top commit.


commit b02fca79e858dc6e6d3209a44fe967f038ac6291
Date:   Wed Jul 9 10:57:33 2014 +0200

I'll take another look at the code. Hopefully I can figure out why the 
messages are getting lost.

Sorry for the delayed response.

--
-- Jason J. Herne (jjhe...@linux.vnet.ibm.com)

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


[libvirt] [v2 1/3] Add gvir_config_capabilities_cpu_get_model()

2014-07-10 Thread Zeeshan Ali (Khattak)
Add a method to get the model of the CPU from capabilities.
---
 libvirt-gconfig/libvirt-gconfig-capabilities-cpu.c | 6 ++
 libvirt-gconfig/libvirt-gconfig-capabilities-cpu.h | 2 ++
 libvirt-gconfig/libvirt-gconfig.sym| 2 ++
 3 files changed, 10 insertions(+)

diff --git a/libvirt-gconfig/libvirt-gconfig-capabilities-cpu.c 
b/libvirt-gconfig/libvirt-gconfig-capabilities-cpu.c
index f4753ff..255c4d7 100644
--- a/libvirt-gconfig/libvirt-gconfig-capabilities-cpu.c
+++ b/libvirt-gconfig/libvirt-gconfig-capabilities-cpu.c
@@ -60,6 +60,12 @@ 
gvir_config_capabilities_cpu_get_arch(GVirConfigCapabilitiesCpu *cpu)
 return gvir_config_object_get_node_content(GVIR_CONFIG_OBJECT(cpu), 
arch);
 }
 
+const gchar *
+gvir_config_capabilities_cpu_get_model(GVirConfigCapabilitiesCpu *cpu)
+{
+return gvir_config_object_get_node_content(GVIR_CONFIG_OBJECT(cpu), 
model);
+}
+
 /**
  * gvir_config_capabilities_cpu_add_feature:
  *
diff --git a/libvirt-gconfig/libvirt-gconfig-capabilities-cpu.h 
b/libvirt-gconfig/libvirt-gconfig-capabilities-cpu.h
index ce3613f..c6c152f 100644
--- a/libvirt-gconfig/libvirt-gconfig-capabilities-cpu.h
+++ b/libvirt-gconfig/libvirt-gconfig-capabilities-cpu.h
@@ -66,6 +66,8 @@ GType gvir_config_capabilities_cpu_get_type(void);
 
 const gchar *
 gvir_config_capabilities_cpu_get_arch(GVirConfigCapabilitiesCpu *cpu);
+const gchar *
+gvir_config_capabilities_cpu_get_model(GVirConfigCapabilitiesCpu *cpu);
 void gvir_config_capabilities_cpu_add_feature(GVirConfigCapabilitiesCpu *cpu,
   GVirConfigCapabilitiesCpuFeature 
*feature);
 GList *
diff --git a/libvirt-gconfig/libvirt-gconfig.sym 
b/libvirt-gconfig/libvirt-gconfig.sym
index 0d33fdb..86dada6 100644
--- a/libvirt-gconfig/libvirt-gconfig.sym
+++ b/libvirt-gconfig/libvirt-gconfig.sym
@@ -689,6 +689,8 @@ global:
 
 LIBVIRT_GCONFIG_0.1.9 {
 global:
+   gvir_config_capabilities_cpu_get_model;
+
gvir_config_capabilities_host_get_secmodels;
 
gvir_config_capabilities_host_secmodel_get_doi;
-- 
1.9.3

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


[libvirt] CPU model API (v2)

2014-07-10 Thread Zeeshan Ali (Khattak)
v2:

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

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


[libvirt] [v2 2/3] Add GVirConfigDomainCpuModel class

2014-07-10 Thread Zeeshan Ali (Khattak)
Add a class to represent 'model' node under domain/cpu.
---
 libvirt-gconfig/Makefile.am|  2 +
 libvirt-gconfig/libvirt-gconfig-domain-cpu-model.c | 94 ++
 libvirt-gconfig/libvirt-gconfig-domain-cpu-model.h | 72 +
 libvirt-gconfig/libvirt-gconfig.h  |  1 +
 libvirt-gconfig/libvirt-gconfig.sym|  5 ++
 5 files changed, 174 insertions(+)
 create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-cpu-model.c
 create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-cpu-model.h

diff --git a/libvirt-gconfig/Makefile.am b/libvirt-gconfig/Makefile.am
index 8a3ff0d..8ad1cc9 100644
--- a/libvirt-gconfig/Makefile.am
+++ b/libvirt-gconfig/Makefile.am
@@ -37,6 +37,7 @@ GCONFIG_HEADER_FILES = \
libvirt-gconfig-domain-controller-usb.h \
libvirt-gconfig-domain-cpu.h \
libvirt-gconfig-domain-cpu-feature.h \
+   libvirt-gconfig-domain-cpu-model.h \
libvirt-gconfig-domain-device.h \
libvirt-gconfig-domain-disk.h \
libvirt-gconfig-domain-disk-driver.h \
@@ -125,6 +126,7 @@ GCONFIG_SOURCE_FILES = \
libvirt-gconfig-domain-controller-usb.c \
libvirt-gconfig-domain-cpu.c \
libvirt-gconfig-domain-cpu-feature.c \
+   libvirt-gconfig-domain-cpu-model.c \
libvirt-gconfig-domain-device.c \
libvirt-gconfig-domain-disk.c \
libvirt-gconfig-domain-disk-driver.c \
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-cpu-model.c 
b/libvirt-gconfig/libvirt-gconfig-domain-cpu-model.c
new file mode 100644
index 000..514a2e3
--- /dev/null
+++ b/libvirt-gconfig/libvirt-gconfig-domain-cpu-model.c
@@ -0,0 +1,94 @@
+/*
+ * libvirt-gconfig-domain-cpu-model.c: libvirt domain CPU model
+ *
+ * Copyright (C) 2014 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * http://www.gnu.org/licenses/.
+ *
+ * Authors: Zeeshan Ali zee...@redhat.com
+ */
+
+#include config.h
+
+#include libvirt-gconfig/libvirt-gconfig.h
+#include libvirt-gconfig/libvirt-gconfig-private.h
+
+#define GVIR_CONFIG_DOMAIN_CPU_MODEL_GET_PRIVATE(obj) \
+(G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_CONFIG_TYPE_DOMAIN_CPU_MODEL, 
GVirConfigDomainCpuModelPrivate))
+
+struct _GVirConfigDomainCpuModelPrivate
+{
+gboolean unused;
+};
+
+G_DEFINE_TYPE(GVirConfigDomainCpuModel, gvir_config_domain_cpu_model, 
GVIR_CONFIG_TYPE_OBJECT);
+
+static void 
gvir_config_domain_cpu_model_class_init(GVirConfigDomainCpuModelClass *klass)
+{
+g_type_class_add_private(klass, sizeof(GVirConfigDomainCpuModelPrivate));
+}
+
+static void gvir_config_domain_cpu_model_init(GVirConfigDomainCpuModel *model)
+{
+g_debug(Init GVirConfigDomainCpuModel=%p, model);
+
+model-priv = GVIR_CONFIG_DOMAIN_CPU_MODEL_GET_PRIVATE(model);
+}
+
+GVirConfigDomainCpuModel *gvir_config_domain_cpu_model_new(void)
+{
+GVirConfigObject *object;
+
+object = gvir_config_object_new(GVIR_CONFIG_TYPE_DOMAIN_CPU_MODEL,
+model,
+NULL);
+
+return GVIR_CONFIG_DOMAIN_CPU_MODEL(object);
+}
+
+GVirConfigDomainCpuModel *
+gvir_config_domain_cpu_model_new_from_xml(const gchar *xml, GError **error)
+{
+GVirConfigObject *object;
+
+object = gvir_config_object_new_from_xml(GVIR_CONFIG_TYPE_DOMAIN_CPU_MODEL,
+ model,
+ NULL,
+ xml,
+ error);
+
+return GVIR_CONFIG_DOMAIN_CPU_MODEL(object);
+}
+
+void
+gvir_config_domain_cpu_model_set_name(GVirConfigDomainCpuModel *model,
+  const gchar *name)
+{
+g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_CPU_MODEL(model));
+
+gvir_config_object_set_node_content
+(GVIR_CONFIG_OBJECT(model),
+ NULL,
+ name);
+}
+
+const gchar *
+gvir_config_domain_cpu_model_get_name(GVirConfigDomainCpuModel *model)
+{
+g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_CPU_MODEL(model), NULL);
+
+return 

[libvirt] [v2 3/3] Add gvir_config_domain_cpu_set_model()

2014-07-10 Thread Zeeshan Ali (Khattak)
Add a method to set model of domain CPU.
---
 libvirt-gconfig/libvirt-gconfig-domain-cpu.c | 11 +++
 libvirt-gconfig/libvirt-gconfig-domain-cpu.h |  4 
 libvirt-gconfig/libvirt-gconfig.sym  |  2 ++
 3 files changed, 17 insertions(+)

diff --git a/libvirt-gconfig/libvirt-gconfig-domain-cpu.c 
b/libvirt-gconfig/libvirt-gconfig-domain-cpu.c
index e7b9575..0037763 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain-cpu.c
+++ b/libvirt-gconfig/libvirt-gconfig-domain-cpu.c
@@ -136,3 +136,14 @@ void gvir_config_domain_cpu_set_mode(GVirConfigDomainCpu 
*cpu,
  mode, GVIR_CONFIG_TYPE_DOMAIN_CPU_MODE, mode,
  NULL);
 }
+
+void gvir_config_domain_cpu_set_model(GVirConfigDomainCpu *cpu,
+  GVirConfigDomainCpuModel *model)
+{
+g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_CPU(cpu));
+g_return_if_fail(model == NULL || GVIR_CONFIG_IS_DOMAIN_CPU_MODEL(model));
+
+gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(cpu),
+  model,
+  GVIR_CONFIG_OBJECT(model));
+}
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-cpu.h 
b/libvirt-gconfig/libvirt-gconfig-domain-cpu.h
index 7efb7eb..f7c0a93 100644
--- a/libvirt-gconfig/libvirt-gconfig-domain-cpu.h
+++ b/libvirt-gconfig/libvirt-gconfig-domain-cpu.h
@@ -28,6 +28,8 @@
 #ifndef __LIBVIRT_GCONFIG_DOMAIN_CPU_H__
 #define __LIBVIRT_GCONFIG_DOMAIN_CPU_H__
 
+#include libvirt-gconfig/libvirt-gconfig-domain-cpu-model.h
+
 G_BEGIN_DECLS
 
 #define GVIR_CONFIG_TYPE_DOMAIN_CPU
(gvir_config_domain_cpu_get_type ())
@@ -80,6 +82,8 @@ GVirConfigDomainCpuMatchPolicy
 gvir_config_domain_cpu_get_match_policy(GVirConfigDomainCpu *cpu);
 void gvir_config_domain_cpu_set_mode(GVirConfigDomainCpu *cpu,
  GVirConfigDomainCpuMode mode);
+void gvir_config_domain_cpu_set_model(GVirConfigDomainCpu *cpu,
+  GVirConfigDomainCpuModel *model);
 GVirConfigDomainCpuMode
 gvir_config_domain_cpu_get_mode(GVirConfigDomainCpu *cpu);
 
diff --git a/libvirt-gconfig/libvirt-gconfig.sym 
b/libvirt-gconfig/libvirt-gconfig.sym
index 4f64820..6d3fe90 100644
--- a/libvirt-gconfig/libvirt-gconfig.sym
+++ b/libvirt-gconfig/libvirt-gconfig.sym
@@ -707,6 +707,8 @@ global:
gvir_config_domain_cpu_model_get_type;
gvir_config_domain_cpu_model_new;
gvir_config_domain_cpu_model_set_name;
+
+   gvir_config_domain_cpu_set_model;
 } LIBVIRT_GCONFIG_0.1.8;
 
 #  define new API here using predicted next version number 
-- 
1.9.3

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


Re: [libvirt] [PREPOST 01/17] src/xenxs:Introduce function

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

 Introduce function xenParseXMGeneral(virConfPtr conf, ...);
 This function parses the xm general options such as uuid and name

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

 diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
 index 25a042d..1953a85 100644
 --- a/src/xenxs/xen_xm.c
 +++ b/src/xenxs/xen_xm.c
 @@ -253,44 +253,25 @@ xenXMConfigGetUUID(virConfPtr conf, const char *name, 
 unsigned char *uuid)
   * Turn a config record into a lump of XML describing the
   * domain, suitable for later feeding for virDomainCreateXML
   */
 -virDomainDefPtr
 -xenParseXM(virConfPtr conf, int xendConfigVersion,
 -   virCapsPtr caps)
 +static int xenParseXMGeneral(virConfPtr conf, virDomainDefPtr def,
 +  virCapsPtr caps)
   

Eric already mentioned the indentation, but I'll mention the name bugs
me a bit :).  Other names that come to mind are xenParseXMBasic,
xenParseXMMetadata, and xenParseXMGeneralMeta.  I tend to prefer the
last one.

Regards,
Jim

  {
 +
 +const char *defaultMachine;
  const char *str;
  int hvm = 0;
 -int val;
 -virConfValuePtr list;
 -virDomainDefPtr def = NULL;
 -virDomainDiskDefPtr disk = NULL;
 -virDomainNetDefPtr net = NULL;
 -virDomainGraphicsDefPtr graphics = NULL;
 -virDomainHostdevDefPtr hostdev = NULL;
 -size_t i;
 -const char *defaultMachine;
 -int vmlocaltime = 0;
 -unsigned long count;
 -char *script = NULL;
 -char *listenAddr = NULL;
 -
 -if (VIR_ALLOC(def)  0)
 -return NULL;
 -
 -def-virtType = VIR_DOMAIN_VIRT_XEN;
 -def-id = -1;
  
  if (xenXMConfigCopyString(conf, name, def-name)  0)
 -goto cleanup;
 +return -1;
  if (xenXMConfigGetUUID(conf, uuid, def-uuid)  0)
 -goto cleanup;
 -
 +return -1;
  
  if ((xenXMConfigGetString(conf, builder, str, linux) == 0) 
  STREQ(str, hvm))
  hvm = 1;
  
  if (VIR_STRDUP(def-os.type, hvm ? hvm : xen)  0)
 -goto cleanup;
 +return -1;
  
  def-os.arch =
  virCapabilitiesDefaultGuestArch(caps,
 @@ -300,7 +281,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  virReportError(VIR_ERR_INTERNAL_ERROR,
 _(no supported architecture for os type '%s'),
 def-os.type);
 -goto cleanup;
 +return -1;
  }
  
  defaultMachine = virCapabilitiesDefaultGuestMachine(caps,
 @@ -309,9 +290,37 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  
 virDomainVirtTypeToString(def-virtType));
  if (defaultMachine != NULL) {
  if (VIR_STRDUP(def-os.machine, defaultMachine)  0)
 -goto cleanup;
 +return -1;
  }
 +return 0;
 +}
 +virDomainDefPtr
 +xenParseXM(virConfPtr conf, int xendConfigVersion,
 +   virCapsPtr caps)
 +{
 +const char *str;
 +int hvm = 0;
 +int val;
 +virConfValuePtr list;
 +virDomainDefPtr def = NULL;
 +virDomainDiskDefPtr disk = NULL;
 +virDomainNetDefPtr net = NULL;
 +virDomainGraphicsDefPtr graphics = NULL;
 +virDomainHostdevDefPtr hostdev = NULL;
 +size_t i;
 +int vmlocaltime = 0;
 +unsigned long count;
 +char *script = NULL;
 +char *listenAddr = NULL;
 +
 +if (VIR_ALLOC(def)  0)
 +return NULL;
  
 +def-virtType = VIR_DOMAIN_VIRT_XEN;
 +def-id = -1;
 +if (xenParseXMGeneral(conf, def, caps)  0)
 +goto cleanup;
 +hvm = (STREQ(def-os.type, hvm));
  if (hvm) {
  const char *boot;
  if (xenXMConfigCopyString(conf, kernel, def-os.loader)  0)
   

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


Re: [libvirt] [PREPOST 01/17] src/xenxs:Introduce function

2014-07-10 Thread Jim Fehlig
Eric Blake wrote:
 On 07/10/2014 07:42 AM, David Kiarie wrote:
   
 From: Kiarie Kahurani davidkiar...@gmail.com

 Introduce function xenParseXMGeneral(virConfPtr conf, ...);
 This function parses the xm general options such as uuid and name
 

 [meta-comment]:

 When posting a series, it's useful to post a cover letter (0/17) to
 which all patches reply to, rather than having 2-17 reply to 1/17 with
 no lead-in on the series as a whole.
   

David, you can add '--cover-letter --annotate' to your git send-email
command to have git generate the cover letter and let you edit it before
sending.

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

 diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
 index 25a042d..1953a85 100644
 --- a/src/xenxs/xen_xm.c
 +++ b/src/xenxs/xen_xm.c
 @@ -253,44 +253,25 @@ xenXMConfigGetUUID(virConfPtr conf, const char *name, 
 unsigned char *uuid)
   * Turn a config record into a lump of XML describing the
   * domain, suitable for later feeding for virDomainCreateXML
   */
 -virDomainDefPtr
 -xenParseXM(virConfPtr conf, int xendConfigVersion,
 -   virCapsPtr caps)
 +static int xenParseXMGeneral(virConfPtr conf, virDomainDefPtr def,
 +  virCapsPtr caps)
 

 Indentation is off; per our style, this should be:

 static int
 xenParseXMGeneral(virConfPtr conf, virDomainDefPtr def,
   virCapsPtr caps)


   
 +}
 +virDomainDefPtr
 

 2 blank lines between functions.

   
 +xenParseXM(virConfPtr conf, int xendConfigVersion,
 +   virCapsPtr caps)
 

 Indentation is off; the second line should line up to the character
 after the ( of the first line.

 Otherwise, this looks like it is just a refactoring of splitting out a
 helper function for later reuse.
   

David is looking into supporting Xen's xl config format in
domxml-{from,to}-native.  For the most part, xm is a subset of xl, so a
lot of the xm parsing code can be used by the xl parser.

David, this type of info would be useful in a cover letter, so reviewers
know what all the code motion is about :-).

Regards,
Jim

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


Re: [libvirt] [PREPOST 02/17] src/xenxs:Introduce function

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

 Introduce function xenParseXMMem(virConfPtr conf,);
 This function parses the xm memory options

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

 diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
 index 1953a85..dc840e5 100644
 --- a/src/xenxs/xen_xm.c
 +++ b/src/xenxs/xen_xm.c
 @@ -294,6 +294,21 @@ static int xenParseXMGeneral(virConfPtr conf, 
 virDomainDefPtr def,
  }
  return 0;
  }
 +static int xenParseXMMem(virConfPtr conf, virDomainDefPtr def)
   

This file has a mixture of 1 and 2 blank lines between functions, but 2
are preferred in new code.

Regards,
Jim

 +{
 +if (xenXMConfigGetULongLong(conf, memory, def-mem.cur_balloon,
 +MIN_XEN_GUEST_SIZE * 2)  0)
 +return -1;
 +
 +if (xenXMConfigGetULongLong(conf, maxmem, def-mem.max_balloon,
 +def-mem.cur_balloon)  0)
 +return -1;
 +
 +def-mem.cur_balloon *= 1024;
 +def-mem.max_balloon *= 1024;
 +
 +return 0;
 +}
  virDomainDefPtr
  xenParseXM(virConfPtr conf, int xendConfigVersion,
 virCapsPtr caps)
 @@ -372,18 +387,9 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  goto cleanup;
  }
  }
 -
 -if (xenXMConfigGetULongLong(conf, memory, def-mem.cur_balloon,
 -MIN_XEN_GUEST_SIZE * 2)  0)
 -goto cleanup;
 -
 -if (xenXMConfigGetULongLong(conf, maxmem, def-mem.max_balloon,
 -def-mem.cur_balloon)  0)
 +if (xenParseXMMem(conf, def)  0)
  goto cleanup;
  
 -def-mem.cur_balloon *= 1024;
 -def-mem.max_balloon *= 1024;
 -
  if (xenXMConfigGetULong(conf, vcpus, count, 1)  0 ||
  MAX_VIRT_CPUS  count)
  goto cleanup;
   

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


Re: [libvirt] [PREPOST 03/17] src/xenxm:Refactor network config parsing code

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

 I introduced the function
   xenFormatXMVif(virConfPtr conf, ..);
 which parses network configuration

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

 diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
 index dc840e5..26ebd5a 100644
 --- a/src/xenxs/xen_xm.c
 +++ b/src/xenxs/xen_xm.c
 @@ -309,6 +309,159 @@ static int xenParseXMMem(virConfPtr conf, 
 virDomainDefPtr def)
  
  return 0;
  }
 +static int xenParseXMVif(virConfPtr conf, virDomainDefPtr def)
   

Same comment here about blank lines between functions.  I won't bore you
with repeating myself in all the patches.  Also, remember Eric's comment
about function return type on one line and name and params on another.  
E.g.

static int
xenParseXMVif(virConfPtr conf, virDomainDefPtr def)
{
...
}
 
 +{
 +char *script = NULL;
 +virDomainNetDefPtr net = NULL;
 +virConfValuePtr list = virConfGetValue(conf, vif);
   

Style is to have a blank line after local variable declarations.

I think you should also define a local variable ret, initialized to -1.

 +if (list  list-type == VIR_CONF_LIST) {
 +list = list-list;
 +while (list) {
 +char model[10];
 +char type[10];
 +char ip[16];
 +char mac[18];
 +char bridge[50];
 +char vifname[50];
 +char *key;
 +
 +bridge[0] = '\0';
 +mac[0] = '\0';
 +ip[0] = '\0';
 +model[0] = '\0';
 +type[0] = '\0';
 +vifname[0] = '\0';
 +
 +if ((list-type != VIR_CONF_STRING) || (list-str == NULL))
 +goto skipnic;
 +
 +key = list-str;
 +while (key) {
 +char *data;
 +char *nextkey = strchr(key, ',');
 +
 +if (!(data = strchr(key, '=')))
 +goto skipnic;
 +data++;
 +
 +if (STRPREFIX(key, mac=)) {
 +int len = nextkey ? (nextkey - data) : sizeof(mac) - 1;
 +if (virStrncpy(mac, data, len, sizeof(mac)) == NULL) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(MAC address %s too big for 
 destination),
 +   data);
 +goto skipnic;
 +}
 +} else if (STRPREFIX(key, bridge=)) {
 +int len = nextkey ? (nextkey - data) : sizeof(bridge) - 
 1;
 +if (virStrncpy(bridge, data, len, sizeof(bridge)) == 
 NULL) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(Bridge %s too big for 
 destination),
 +   data);
 +goto skipnic;
 +}
 +} else if (STRPREFIX(key, script=)) {
 +int len = nextkey ? (nextkey - data) : strlen(data);
 +VIR_FREE(script);
 +if (VIR_STRNDUP(script, data, len)  0)
 +goto cleanup;
 +} else if (STRPREFIX(key, model=)) {
 +int len = nextkey ? (nextkey - data) : sizeof(model) - 1;
 +if (virStrncpy(model, data, len, sizeof(model)) == NULL) 
 {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(Model %s too big for 
 destination), data);
 +goto skipnic;
 +}
 +} else if (STRPREFIX(key, type=)) {
 +int len = nextkey ? (nextkey - data) : sizeof(type) - 1;
 +if (virStrncpy(type, data, len, sizeof(type)) == NULL) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(Type %s too big for destination), 
 data);
 +goto skipnic;
 +}
 +} else if (STRPREFIX(key, vifname=)) {
 +int len = nextkey ? (nextkey - data) : sizeof(vifname) - 
 1;
 +if (virStrncpy(vifname, data, len, sizeof(vifname)) == 
 NULL) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(Vifname %s too big for 
 destination),
 +   data);
 +goto skipnic;
 +}
 +} else if (STRPREFIX(key, ip=)) {
 +int len = nextkey ? (nextkey - data) : sizeof(ip) - 1;
 +if (virStrncpy(ip, data, len, sizeof(ip)) == NULL) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(IP %s 

Re: [libvirt] [PREPOST 04/17] src/xenxs:Refactor code parsing time and event actions config

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

 Introduce the functions
  xenParseXMTimeOffset(...);
  xenParseXMEventActions(..);
   

These aren't related and should be split into two patches IMO.

Regards,
Jim

 These parse the time config and Event actions configs
 respectively

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

 diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
 index 26ebd5a..e30ab9c 100644
 --- a/src/xenxs/xen_xm.c
 +++ b/src/xenxs/xen_xm.c
 @@ -462,6 +462,73 @@ cleanup:
  VIR_FREE(script);
  return -1;
  }
 +static int xenParseXMTimeOffset(virConfPtr conf, virDomainDefPtr def,
 +int xendConfigVersion)
 +{
 +int vmlocaltime;
 +
 +if (xenXMConfigGetBool(conf, localtime, vmlocaltime, 0)  0)
 +return -1;
 +
 +if (STREQ(def-os.type, hvm)) {
 +/* only managed HVM domains since 3.1.0 have persistent 
 rtc_timeoffset */
 +if (xendConfigVersion  XEND_CONFIG_VERSION_3_1_0) {
 +if (vmlocaltime)
 +def-clock.offset = VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME;
 +else
 +def-clock.offset = VIR_DOMAIN_CLOCK_OFFSET_UTC;
 +def-clock.data.utc_reset = true;
 +} else {
 +unsigned long rtc_timeoffset;
 +def-clock.offset = VIR_DOMAIN_CLOCK_OFFSET_VARIABLE;
 +if (xenXMConfigGetULong(conf, rtc_timeoffset, rtc_timeoffset, 
 0)  0)
 +return -1;
 +def-clock.data.variable.adjustment = (int)rtc_timeoffset;
 +def-clock.data.variable.basis = vmlocaltime ?
 +VIR_DOMAIN_CLOCK_BASIS_LOCALTIME :
 +VIR_DOMAIN_CLOCK_BASIS_UTC;
 +}
 +} else {
 +/* PV domains do not have an emulated RTC and the offset is fixed. */
 +def-clock.offset = vmlocaltime ?
 +VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME :
 +VIR_DOMAIN_CLOCK_OFFSET_UTC;
 +def-clock.data.utc_reset = true;
 +} /* !hvm */
 +
 +return 0;
 +}
 +static int xenParseXMEventsActions(virConfPtr conf, virDomainDefPtr def)
 +{
 +const char *str;
 +
 +if (xenXMConfigGetString(conf, on_poweroff, str, destroy)  0)
 +return -1;
 +if ((def-onPoweroff = virDomainLifecycleTypeFromString(str))  0) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(unexpected value %s for on_poweroff), str);
 +return -1;
 +}
 +
 +if (xenXMConfigGetString(conf, on_reboot, str, restart)  0)
 +return -1;
 +if ((def-onReboot = virDomainLifecycleTypeFromString(str))  0) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(unexpected value %s for on_reboot), str);
 +return -1;
 +}
 +
 +if (xenXMConfigGetString(conf, on_crash, str, restart)  0)
 +return -1;
 +if ((def-onCrash = virDomainLifecycleCrashTypeFromString(str))  0) {
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(unexpected value %s for on_crash), str);
 +return -1;
 +}
 +
 +return 0;
 +
 +}
  virDomainDefPtr
  xenParseXM(virConfPtr conf, int xendConfigVersion,
 virCapsPtr caps)
 @@ -476,7 +543,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  virDomainGraphicsDefPtr graphics = NULL;
  virDomainHostdevDefPtr hostdev = NULL;
  size_t i;
 -int vmlocaltime = 0;
  unsigned long count;
  char *script = NULL;
  char *listenAddr = NULL;
 @@ -556,32 +622,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  if (str  (virBitmapParse(str, 0, def-cpumask, 4096)  0))
  goto cleanup;
  
 -if (xenXMConfigGetString(conf, on_poweroff, str, destroy)  0)
 -goto cleanup;
 -if ((def-onPoweroff = virDomainLifecycleTypeFromString(str))  0) {
 -virReportError(VIR_ERR_INTERNAL_ERROR,
 -   _(unexpected value %s for on_poweroff), str);
 -goto cleanup;
 -}
 -
 -if (xenXMConfigGetString(conf, on_reboot, str, restart)  0)
 -goto cleanup;
 -if ((def-onReboot = virDomainLifecycleTypeFromString(str))  0) {
 -virReportError(VIR_ERR_INTERNAL_ERROR,
 -   _(unexpected value %s for on_reboot), str);
 -goto cleanup;
 -}
 -
 -if (xenXMConfigGetString(conf, on_crash, str, restart)  0)
 -goto cleanup;
 -if ((def-onCrash = virDomainLifecycleCrashTypeFromString(str))  0) {
 -virReportError(VIR_ERR_INTERNAL_ERROR,
 -   _(unexpected value %s for on_crash), str);
 -goto cleanup;
 -}
 -
 -
 -
  if (hvm) {
  if (xenXMConfigGetBool(conf, pae, val, 0)  0)
  goto cleanup;
 @@ -621,35 +661,6 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
  def-clock.timers[0] = 

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

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

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

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

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

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 6a2e760..1e1e53f 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -229,6 +229,16 @@ networkDnsmasqLeaseFileNameCustom(const char *bridge)
 }
 
 static char *
+networkDnsmasqPseudoLeaseHelperName(const char *bridge)
+{
+char *pseudo_leasehelper;
+
+ignore_value(virAsprintf(pseudo_leasehelper, %s/%s.helper,
+ driverState-dnsmasqStateDir, bridge));
+return pseudo_leasehelper;
+}
+
+static char *
 networkDnsmasqConfigFileName(const char *netname)
 {
 char *conffile;
@@ -1261,6 +1271,7 @@ networkBuildDhcpDaemonCommandLine(virNetworkObjPtr 
network,
 char *configfile = NULL;
 char *configstr = NULL;
 char *leaseshelper_path = NULL;
+char *pseudo_leaseshelper_path = NULL;
 
 network-dnsmasqPid = -1;
 
@@ -1273,6 +1284,11 @@ networkBuildDhcpDaemonCommandLine(virNetworkObjPtr 
network,
 if (!(configfile = networkDnsmasqConfigFileName(network-def-name)))
 goto cleanup;
 
+/* construct symlink name */
+if (!(pseudo_leaseshelper_path
+  = networkDnsmasqPseudoLeaseHelperName(network-def-bridge)))
+goto cleanup;
+
 /* Write the file */
 if (virFileWriteStr(configfile, configstr, 0600)  0) {
 virReportSystemError(errno,
@@ -1287,9 +1303,30 @@ networkBuildDhcpDaemonCommandLine(virNetworkObjPtr 
network,
   LIBEXECDIR)))
 goto cleanup;
 
+/* Symlink technique for dnsmasq lease file is needed so that libvirt
+ * can have complete control over the handling of leases database */
+
+/* Remove unwanted, old symlink */
+if (unlink(pseudo_leaseshelper_path)  0 
+errno != ENOENT  errno != ENOTDIR) {
+virReportSystemError(errno,
+ _(Failed to delete symlink '%s'),
+ pseudo_leaseshelper_path);
+goto cleanup;
+}
+
+/* Create a new symlink */
+if (symlink(leaseshelper_path, pseudo_leaseshelper_path)  0) {
+virReportSystemError(errno,
+ _(Failed to create symlink '%s' to '%s'),
+ leaseshelper_path, pseudo_leaseshelper_path);
+goto cleanup;
+}
+
 cmd = virCommandNew(dnsmasqCapsGetBinaryPath(caps));
 virCommandAddArgFormat(cmd, --conf-file=%s, configfile);
-virCommandAddArgFormat(cmd, --dhcp-script=%s, leaseshelper_path);
+virCommandAddArgFormat(cmd, --dhcp-script=%s, pseudo_leaseshelper_path);
+virCommandAddArgFormat(cmd, --leasefile-ro);
 
 *cmdout = cmd;
 ret = 0;
@@ -3432,6 +3469,10 @@ networkGetDHCPLeases(virNetworkPtr network,
 goto error;
 }
 
+/* Ignore server-duid. It's not part of a lease */
+if (virJSONValueObjectHasKey(lease_tmp, server-duid))
+continue;
+
 if (!(mac_tmp = virJSONValueObjectGetString(lease_tmp, 
mac-address))) {
 /* leaseshelper program guarantees that lease will be stored only 
if
  * mac-address is known otherwise not */
diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c
index e4b5283..3d6c773 100644
--- a/src/network/leaseshelper.c
+++ b/src/network/leaseshelper.c
@@ -50,6 +50,13 @@
  */
 #define VIR_NETWORK_DHCP_LEASE_FILE_SIZE_MAX (32 * 1024 * 1024)
 
+/*
+ * Use this when passing possibly-NULL strings to printf-a-likes.
+ * Required for unkown parameters during init call.
+ */
+# define EMPTY_STR(s) ((s) ? (s) : *)
+
+
 static const char *program_name;
 
 /* Display version information. */
@@ -65,7 +72,7 @@ usage(int status)
 if 

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

2014-07-10 Thread chenhanx...@cn.fujitsu.com


 -Original Message-
 From: Ján Tomko [mailto:jto...@redhat.com]
 Sent: Thursday, July 10, 2014 9:40 PM
 
  -if (virDomainSetMemoryEnsureACL(dom-conn, vm-def)  0)
  +if (virDomainSetMemoryFlagsEnsureACL(dom-conn, vm-def, flags)  0)
  +goto cleanup;
  +
  +if (!(caps = virLXCDriverGetCapabilities(driver, false)))
  +goto cleanup;
  +
  +if (virDomainLiveConfigHelperMethod(caps, driver-xmlopt, vm, flags,
  +persistentDef)  0)
   goto cleanup;
 
 
   if (newmem  vm-def-mem.max_balloon) {
 
 This check should only be done for AFFECT_LIVE.
 For AFFECT_CONFIG it needs to be checked against the max_balloon value from
 the persistent definition.
 
Oops, my fault.
Thanks for your comments

- Chen


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

Re: [libvirt] [PATCH] bhyve: reconnect to domains after libvirtd restart

2014-07-10 Thread Roman Bogorodskiy
  Roman Bogorodskiy wrote:

   Roman Bogorodskiy wrote:
 
  Try to reconnect to the running domains after libvirtd restart. To
  achieve that, do:

ping?

Roman Bogorodskiy

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