Re: [libvirt] [PATCH 4/5] conf: add options for disabling segment offloading

2014-09-17 Thread Ján Tomko
On 09/16/2014 12:24 AM, John Ferlan wrote:
 
 
 On 09/11/2014 07:43 AM, Ján Tomko wrote:
 Add the following attributes:
 csum, gso, guest_tso4, guest_tso6, guest_ecn
 to the driver element of network interface
 which control the virtio-net device properties
 of the same names.
 ---
  docs/formatdomain.html.in  | 27 
  docs/schemas/domaincommon.rng  | 25 +++
  src/conf/domain_conf.c | 81 
 ++
  src/conf/domain_conf.h |  5 ++
  .../qemuxml2argv-net-virtio-disable-offloads.xml   | 32 +
  tests/qemuxml2xmltest.c|  1 +
  6 files changed, 171 insertions(+)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml

 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
 index a2ea758..5b2758a 100644
 --- a/docs/formatdomain.html.in
 +++ b/docs/formatdomain.html.in
 @@ -3847,6 +3847,12 @@ qemu-kvm -net nic,model=? /dev/null
lt;model type='virtio'/gt;
blt;driver name='vhost' txmode='iothread' ioeventfd='on' 
 event_idx='off' queues='5'/gt;/b
  lt;/interfacegt;
 +lt;interface type='network'gt;
 +  lt;source network='default'/gt;
 +  lt;target dev='vnet2'/gt;
 +  lt;model type='virtio'/gt;
 +  blt;driver csum='off' gso='off' guest_tso4='off' guest_tso6='off' 
 guest_ecn='off'/gt;/b
 +lt;/interfacegt;
 
 This doesn't require a driver name='' value?
 

AFAIK only queues require name='vhost'.

lt;/devicesgt;
.../pre
  
 @@ -3949,6 +3955,27 @@ qemu-kvm -net nic,model=? /dev/null
  processor, resulting in much higher throughput.
  span class=sinceSince 1.0.6 (QEMU and KVM only)/span
/dd
 +  dtcodecsum/code/dt
 +  dd
 +The codecsum/code attribute with possible values codeon/code
 +and codeoff/code controls host-side support for packets with
 +partial checksum values.
 +span class=sinceSince 1.2.9 (QEMU only)/spanbr/br/
 +
 +bIn general you should leave this option alone, unless you
 +are very certain you know what you are doing./b
 +  /dd
 +  dtsegment offloading options/dt
 +  dd
 +The attributes codegso/code, codeguest_tso4/code,
 +codeguest_tso6/code, codeguest_ecn/code with possible
 +values of codeon/code and codeoff/code can be used
 +to tune segment offloading.
 +span class=sinceSince 1.2.9 (QEMU only)/spanbr/br/
 +
 +bIn general you should leave this option alone, unless you
 +are very certain you know what you are doing./b
 
 s/this option/these options/
 
 [oh - I'm having a flashback to something similar I had to do at my
 former employer with their virtualization switch software...  these got
 enabled by some application and caused shall we say significant
 performance issues, especially for older drivers.  That particular
 software was loaded/started after the virtualization network switch and
 thus reset what our code had done... Bugger to find as it embedded in
 some output from some network command that was executed rarely in our
 vswitch network daemon layer]
 
 Anyway, I understand it's desired to not say much about them, but is
 there any need to say what the defaults are? Furthermore, does one
 domain's setting affect other domains?  I guess my curiosity is more is
 this a domain function or an interface (host) setting. We may want to
 indicate that here... Is the difference dependent upon the driver name?
 

This is a per (guest) interface setting, I'm not aware of it affecting other
interfaces or domains. Perhaps the 'leave this option alone' was too harsh.
I'll add the defaults before pushing/sending another version.

Jan

 I know from my previous experience it had a wider affect, but that code
 had a very different implementation. The vswitch was separate from the
 guest as a host process to which guests could configure ports. The
 vswitch software had the configuration magic to the lower level network
 driver(s) which is where the tso/cko, etc. were managed in the kernel.
 The equivalent of 'em0', 'eth0', etc - the physical NIC. A vswitch was
 tied to a particular physical NIC. So any changes to the pNIC cast a
 wide net, which is why other software setting TSO/CKO options on the
 same pNIC our vswitch used after our code disabled it caused all sorts
 of issues.  (Just so you understand why I'm asking the question).
 
 +  /dd
  /dl
  
  h5a name=elementsNICSTargetOverrideOverriding the target 
 element/a/h5




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/5] conf: add options for disabling segment offloading

2014-09-17 Thread Ján Tomko
On 09/16/2014 12:30 AM, Eric Blake wrote:
 On 09/11/2014 05:43 AM, Ján Tomko wrote:
 Add the following attributes:
 csum, gso, guest_tso4, guest_tso6, guest_ecn
 to the driver element of network interface
 which control the virtio-net device properties
 of the same names.
 ---
  docs/formatdomain.html.in  | 27 
  docs/schemas/domaincommon.rng  | 25 +++
  src/conf/domain_conf.c | 81 
 ++
  src/conf/domain_conf.h |  5 ++
  .../qemuxml2argv-net-virtio-disable-offloads.xml   | 32 +
  tests/qemuxml2xmltest.c|  1 +
  6 files changed, 171 insertions(+)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml

 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
 index a2ea758..5b2758a 100644
 --- a/docs/formatdomain.html.in
 +++ b/docs/formatdomain.html.in
 @@ -3847,6 +3847,12 @@ qemu-kvm -net nic,model=? /dev/null
lt;model type='virtio'/gt;
blt;driver name='vhost' txmode='iothread' ioeventfd='on' 
 event_idx='off' queues='5'/gt;/b
  lt;/interfacegt;
 +lt;interface type='network'gt;
 +  lt;source network='default'/gt;
 +  lt;target dev='vnet2'/gt;
 +  lt;model type='virtio'/gt;
 +  blt;driver csum='off' gso='off' guest_tso4='off' guest_tso6='off' 
 guest_ecn='off'/gt;/b
 +lt;/interfacegt;
 
 Are we stuck with names with underscores in our XML?  I'm still not sure
 if we've come up with the best naming for exposing all these knobs.

I'd rather not mix underscores (event_idx) and other word separators in the
same element.

Alternatively, we could do something like:
driver csum='off' gso='off'
  guest tso4='off' tso6='off' ecn='off'/
/driver
to get rid of the multi-word attributes, but I like the underscores better
because they match QEMU arguments.

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 1/1] qemu: Add support for multiple versions of 'pseries' machine type

2014-09-17 Thread Pradipta Kumar Banerjee
On 09/15/2014 07:46 PM, Ján Tomko wrote:
 On 09/13/2014 05:28 PM, Pradipta Kr. Banerjee wrote:
 qemu: Add support for multiple versions of 'pseries' machine type

 qemu for IBM Power processor architecture is adding functionality for
 supporting multiple 'pseries' machine type versions, each with different
 capabilities. This patch is for supporting the same

 Signed-off-by: Pradipta Kr. Banerjee bpra...@in.ibm.com
 ---
  src/qemu/qemu_command.c | 18 +-
  1 file changed, 9 insertions(+), 9 deletions(-)
 
 ACK.
 
 Would you like to add a qemuxml2argvtest for 'pseries-2.1' machine with these
 devices?
As of now there are no changes to qemu command line invocation based on
different machine type. If required for this patch I'll go ahead and add the
test cases for pseries-2.1 and pseries-2.2. pseries-2.2 is the latest which
aliases to pseries. Please do let me know your thoughts.


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


-- 
Regards,
Pradipta Kumar B(bpra...@in.ibm.com)
IBM Systems  Technology Labs,
India.

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


Re: [libvirt] [PATCH 1/1] qemu: Add support for multiple versions of 'pseries' machine type

2014-09-17 Thread Ján Tomko
On 09/17/2014 11:02 AM, Pradipta Kumar Banerjee wrote:
 On 09/15/2014 07:46 PM, Ján Tomko wrote:
 On 09/13/2014 05:28 PM, Pradipta Kr. Banerjee wrote:
 qemu: Add support for multiple versions of 'pseries' machine type

 qemu for IBM Power processor architecture is adding functionality for
 supporting multiple 'pseries' machine type versions, each with different
 capabilities. This patch is for supporting the same

 Signed-off-by: Pradipta Kr. Banerjee bpra...@in.ibm.com
 ---
  src/qemu/qemu_command.c | 18 +-
  1 file changed, 9 insertions(+), 9 deletions(-)

 ACK.

 Would you like to add a qemuxml2argvtest for 'pseries-2.1' machine with these
 devices?
 As of now there are no changes to qemu command line invocation based on
 different machine type. If required for this patch I'll go ahead and add the
 test cases for pseries-2.1 and pseries-2.2.

It is not required for the patch, I just thought a test would help to make
sure that we don't break the old machine types if we ever change something in
the code.

I've pushed the patch now.

 pseries-2.2 is the latest which
 aliases to pseries. Please do let me know your thoughts.

Oh, so it's the other way than on x86_64 where 'pc' is the alias for the
latest pc-i440fx machine (pc-i440fx-2.2 as of now).

Jan



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

[libvirt] [PATCH] qemu: fix crash with shared disks

2014-09-17 Thread Ján Tomko
Commit f36a94f introduced a double free on all success paths
in qemuSharedDeviceEntryInsert.

Only call qemuSharedDeviceEntryFree on the error path and
set entry to NULL before jumping there if the entry already
is in the hash table.

https://bugzilla.redhat.com/show_bug.cgi?id=1142722
---
 src/qemu/qemu_conf.c | 26 --
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index ac10b64..adc6caf 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1011,38 +1011,36 @@ qemuSharedDeviceEntryInsert(virQEMUDriverPtr driver,
 const char *name)
 {
 qemuSharedDeviceEntry *entry = NULL;
-int ret = -1;
 
 if ((entry = virHashLookup(driver-sharedDevices, key))) {
 /* Nothing to do if the shared scsi host device is already
  * recorded in the table.
  */
-if (qemuSharedDeviceEntryDomainExists(entry, name, NULL)) {
-ret = 0;
-goto cleanup;
+if (!qemuSharedDeviceEntryDomainExists(entry, name, NULL)) {
+if (VIR_EXPAND_N(entry-domains, entry-ref, 1)  0 ||
+VIR_STRDUP(entry-domains[entry-ref - 1], name)  0) {
+/* entry is owned by the hash table here */
+entry = NULL;
+goto error;
+}
 }
-
-if (VIR_EXPAND_N(entry-domains, entry-ref, 1)  0 ||
-VIR_STRDUP(entry-domains[entry-ref - 1], name)  0)
-goto cleanup;
 } else {
 if (VIR_ALLOC(entry)  0 ||
 VIR_ALLOC_N(entry-domains, 1)  0 ||
 VIR_STRDUP(entry-domains[0], name)  0)
-goto cleanup;
+goto error;
 
 entry-ref = 1;
 
 if (virHashAddEntry(driver-sharedDevices, key, entry))
-goto cleanup;
+goto error;
 }
 
-ret = 0;
+return 0;
 
- cleanup:
+ error:
 qemuSharedDeviceEntryFree(entry, NULL);
-
-return ret;
+return -1;
 }
 
 
-- 
1.8.5.5

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


Re: [libvirt] [PATCH] domaincaps: Expose UEFI binary path, if it exists

2014-09-17 Thread Laszlo Ersek
Hi Cole,

I'm not subscribed to the list; please CC me on UEFI-related patches.
Michal pinged me to review this one. Some comments:

On 09/17/14 01:52, Cole Robinson wrote:
 Check to see if the UEFI binary mentioned in qemu.conf actually
 exists, and if so expose it in domcapabilities like
 
 loader ...
   value/path/to/ovmf/value
 /loader
 
 We introduce some generic domcaps infrastructure for handling
 a dynamic list of string values, it may be of use for future bits.
 ---
  docs/formatdomaincaps.html.in  |  6 
  docs/schemas/domaincaps.rng| 17 ++---
  src/conf/domain_capabilities.c | 23 
  src/conf/domain_capabilities.h |  8 +
  src/qemu/qemu_driver.c | 24 +
  tests/domaincapsschemadata/domaincaps-full.xml |  1 +
  tests/domaincapstest.c | 49 
 +-
  7 files changed, 115 insertions(+), 13 deletions(-)
 
 diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
 index 34d746d..6959dfe 100644
 --- a/docs/formatdomaincaps.html.in
 +++ b/docs/formatdomaincaps.html.in
 @@ -105,6 +105,7 @@
...
lt;os supported='yes'gt;
  lt;loader supported='yes'gt;
 +  lt;valuegt;/usr/share/OVMF/OVMF_CODE.fdlt;/valuegt;
lt;enum name='type'gt;
  lt;valuegt;romlt;/valuegt;
  lt;valuegt;pflashlt;/valuegt;
 @@ -122,6 +123,11 @@
  pFor the codeloader/code element, the following can occur:/p
  
  dl
 +  dtvalue/dt
 +  ddList of known loader paths. Currently this is only used
 +  to advertise known locations of OVMF binaries for qemu. Binaries
 +  will only be listed if they actually exist on disk./dd
 +
dttype/dt
ddWhether loader is a typical BIOS (coderom/code) or
an UEFI binary (codepflash/code). This refers to
 diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
 index ad8d966..dfdb9b9 100644
 --- a/docs/schemas/domaincaps.rng
 +++ b/docs/schemas/domaincaps.rng
 @@ -46,6 +46,9 @@
  
define name='loader'
  element name='loader'
 +  optional
 +ref name='value'/
 +  /optional
ref name='supported'/
ref name='enum'/
  /element
 @@ -85,6 +88,14 @@
  /element
/define
  
 +  define name='value'
 +zeroOrMore
 +  element name='value'
 +text/
 +  /element
 +/zeroOrMore
 +  /define
 +
define name='supported'
  attribute name='supported'
choice
 @@ -100,11 +111,7 @@
  attribute name='name'
text/
  /attribute
 -zeroOrMore
 -  element name='value'
 -text/
 -  /element
 -/zeroOrMore
 +ref name='value'/
/element
  /zeroOrMore
/define
 diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
 index 5a3c8e7..44e422a 100644
 --- a/src/conf/domain_capabilities.c
 +++ b/src/conf/domain_capabilities.c
 @@ -46,6 +46,15 @@ static int virDomainCapsOnceInit(void)
  
  VIR_ONCE_GLOBAL_INIT(virDomainCaps)
  
 +static void
 +virDomainCapsValuesFree(virDomainCapsValuesPtr values)
 +{
 +size_t i;
 +
 +for (i = 0; i  values-nvalues; i++) {
 +VIR_FREE(values-values[i]);
 +}
 +}
  
  static void
  virDomainCapsDispose(void *obj)
 @@ -54,6 +63,8 @@ virDomainCapsDispose(void *obj)
  
  VIR_FREE(caps-path);
  VIR_FREE(caps-machine);
 +
 +virDomainCapsValuesFree(caps-os.loader.values);
  }

(1) This leaks the caps-os.loader.values.values array. (Which is a
dynamically allocated array of pointers.) virDomainCapsValuesFree()
should VIR_FREE() it too.

  
  
 @@ -156,6 +167,17 @@ virDomainCapsEnumFormat(virBufferPtr buf,
  return ret;
  }
  
 +static void
 +virDomainCapsValuesFormat(virBufferPtr buf,
 +  virDomainCapsValuesPtr values)
 +{
 +size_t i;
 +
 +for (i = 0; i  values-nvalues; i++) {
 +virBufferAsprintf(buf, value%s/value\n, values-values[i]);
 +}
 +}

(2) virBufferEscapeString() would probably useful here; the filename
shouldn't be plainly embedded into an XML fragment.

I'm not sure if we paid attention to this with the nvram patches... Hm
yes; I rechecked Michal's commits now, and they seem to use
virBufferEscapeString().

 +
  #define FORMAT_PROLOGUE(item)   \
  do {\
  virBufferAsprintf(buf,  #item  supported='%s'%s\n, \
 @@ -185,6 +207,7 @@ virDomainCapsLoaderFormat(virBufferPtr buf,
  {
  FORMAT_PROLOGUE(loader);
  
 +virDomainCapsValuesFormat(buf, loader-values);
  ENUM_PROCESS(loader, type, virDomainLoaderTypeToString);
  ENUM_PROCESS(loader, readonly, virTristateBoolTypeToString);
  
 diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
 index 768646b..3d5aaa3 100644
 --- a/src/conf/domain_capabilities.h
 +++ 

Re: [libvirt] [PATCH] domaincaps: Expose UEFI binary path, if it exists

2014-09-17 Thread Michal Privoznik

On 17.09.2014 01:52, Cole Robinson wrote:

Check to see if the UEFI binary mentioned in qemu.conf actually
exists, and if so expose it in domcapabilities like

loader ...
   value/path/to/ovmf/value
/loader

We introduce some generic domcaps infrastructure for handling
a dynamic list of string values, it may be of use for future bits.
---
  docs/formatdomaincaps.html.in  |  6 
  docs/schemas/domaincaps.rng| 17 ++---
  src/conf/domain_capabilities.c | 23 
  src/conf/domain_capabilities.h |  8 +
  src/qemu/qemu_driver.c | 24 +
  tests/domaincapsschemadata/domaincaps-full.xml |  1 +
  tests/domaincapstest.c | 49 +-
  7 files changed, 115 insertions(+), 13 deletions(-)

diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
index 34d746d..6959dfe 100644
--- a/docs/formatdomaincaps.html.in
+++ b/docs/formatdomaincaps.html.in
@@ -105,6 +105,7 @@
...
lt;os supported='yes'gt;
  lt;loader supported='yes'gt;
+  lt;valuegt;/usr/share/OVMF/OVMF_CODE.fdlt;/valuegt;
lt;enum name='type'gt;
  lt;valuegt;romlt;/valuegt;
  lt;valuegt;pflashlt;/valuegt;
@@ -122,6 +123,11 @@
  pFor the codeloader/code element, the following can occur:/p

  dl
+  dtvalue/dt
+  ddList of known loader paths. Currently this is only used
+  to advertise known locations of OVMF binaries for qemu. Binaries
+  will only be listed if they actually exist on disk./dd
+
dttype/dt
ddWhether loader is a typical BIOS (coderom/code) or
an UEFI binary (codepflash/code). This refers to
diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
index ad8d966..dfdb9b9 100644
--- a/docs/schemas/domaincaps.rng
+++ b/docs/schemas/domaincaps.rng
@@ -46,6 +46,9 @@

define name='loader'
  element name='loader'
+  optional
+ref name='value'/
+  /optional
ref name='supported'/


I know it doesn't really matter, but I prefer attributes to be defined 
before any child elements. So I'd move 'supported' a few lines up.



ref name='enum'/
  /element
@@ -85,6 +88,14 @@
  /element
/define

+  define name='value'
+zeroOrMore
+  element name='value'
+text/
+  /element
+/zeroOrMore
+  /define
+
define name='supported'
  attribute name='supported'
choice
@@ -100,11 +111,7 @@
  attribute name='name'
text/
  /attribute
-zeroOrMore
-  element name='value'
-text/
-  /element
-/zeroOrMore
+ref name='value'/
/element
  /zeroOrMore
/define
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 5a3c8e7..44e422a 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -46,6 +46,15 @@ static int virDomainCapsOnceInit(void)

  VIR_ONCE_GLOBAL_INIT(virDomainCaps)

+static void
+virDomainCapsValuesFree(virDomainCapsValuesPtr values)
+{
+size_t i;
+
+for (i = 0; i  values-nvalues; i++) {
+VIR_FREE(values-values[i]);
+}
+}

  static void
  virDomainCapsDispose(void *obj)
@@ -54,6 +63,8 @@ virDomainCapsDispose(void *obj)

  VIR_FREE(caps-path);
  VIR_FREE(caps-machine);
+
+virDomainCapsValuesFree(caps-os.loader.values);
  }


@@ -156,6 +167,17 @@ virDomainCapsEnumFormat(virBufferPtr buf,
  return ret;
  }

+static void
+virDomainCapsValuesFormat(virBufferPtr buf,
+  virDomainCapsValuesPtr values)
+{
+size_t i;
+
+for (i = 0; i  values-nvalues; i++) {
+virBufferAsprintf(buf, value%s/value\n, values-values[i]);
+}
+}
+
  #define FORMAT_PROLOGUE(item)   \
  do {\
  virBufferAsprintf(buf,  #item  supported='%s'%s\n, \
@@ -185,6 +207,7 @@ virDomainCapsLoaderFormat(virBufferPtr buf,
  {
  FORMAT_PROLOGUE(loader);

+virDomainCapsValuesFormat(buf, loader-values);
  ENUM_PROCESS(loader, type, virDomainLoaderTypeToString);
  ENUM_PROCESS(loader, readonly, virTristateBoolTypeToString);

diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index 768646b..3d5aaa3 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -37,6 +37,13 @@ struct _virDomainCapsEnum {
  unsigned int values; /* Bitmask of values supported in the corresponding 
enum */
  };

+typedef struct _virDomainCapsValues virDomainCapsValues;
+typedef virDomainCapsValues *virDomainCapsValuesPtr;
+struct _virDomainCapsValues {
+char **values; /* raw string values */
+size_t nvalues; /* number of strings */
+};


While this works, I'd rename this to virDomainCapsStringValues so that 
it's clear what values do we have in mind. Moreover, if we ever 

[libvirt] [PATCH v2 0/2] Expose UEFI binary path

2014-09-17 Thread Michal Privoznik
This is practically reworked v1 from Cole.

Cole Robinson (1):
  domaincaps: Expose UEFI binary path, if it exists

Michal Privoznik (1):
  qemu_capabilities: Change virQEMUCapsFillDomainCaps signature

 docs/formatdomaincaps.html.in  |  6 ++
 docs/schemas/domaincaps.rng| 17 --
 src/conf/domain_capabilities.c | 29 ++
 src/conf/domain_capabilities.h |  8 +++
 src/qemu/qemu_capabilities.c   | 53 +
 src/qemu/qemu_capabilities.h   |  9 ++-
 src/qemu/qemu_driver.c |  7 ++-
 tests/domaincapsschemadata/domaincaps-full.xml |  2 +
 .../domaincaps-qemu_1.6.50-1.xml   |  1 +
 tests/domaincapstest.c | 66 ++
 10 files changed, 167 insertions(+), 31 deletions(-)

-- 
1.8.5.5

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


[libvirt] [PATCH v2 1/2] qemu_capabilities: Change virQEMUCapsFillDomainCaps signature

2014-09-17 Thread Michal Privoznik
Up till now the virQEMUCapsFillDomainCaps() was type of void as
there was no way for it to fail. This is, however, going to
change in the next commit.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/qemu/qemu_capabilities.c | 25 -
 src/qemu/qemu_capabilities.h |  4 ++--
 src/qemu/qemu_driver.c   |  3 ++-
 tests/domaincapstest.c   | 19 ---
 4 files changed, 32 insertions(+), 19 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 9f8868d..c306899 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3608,7 +3608,7 @@ virQEMUCapsGetDefaultMachine(virQEMUCapsPtr qemuCaps)
 }
 
 
-static void
+static int
 virQEMUCapsFillDomainLoaderCaps(virQEMUCapsPtr qemuCaps,
 virDomainCapsLoaderPtr loader,
 virArch arch)
@@ -3629,10 +3629,11 @@ virQEMUCapsFillDomainLoaderCaps(virQEMUCapsPtr qemuCaps,
 VIR_DOMAIN_CAPS_ENUM_SET(loader-readonly,
  VIR_TRISTATE_BOOL_YES,
  VIR_TRISTATE_BOOL_NO);
+return 0;
 }
 
 
-static void
+static int
 virQEMUCapsFillDomainOSCaps(virQEMUCapsPtr qemuCaps,
 virDomainCapsOSPtr os,
 virArch arch)
@@ -3640,11 +3641,13 @@ virQEMUCapsFillDomainOSCaps(virQEMUCapsPtr qemuCaps,
 virDomainCapsLoaderPtr loader = os-loader;
 
 os-device.supported = true;
-virQEMUCapsFillDomainLoaderCaps(qemuCaps, loader, arch);
+if (virQEMUCapsFillDomainLoaderCaps(qemuCaps, loader, arch)  0)
+return -1;
+return 0;
 }
 
 
-static void
+static int
 virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps,
 virDomainCapsDeviceDiskPtr disk)
 {
@@ -3667,10 +3670,11 @@ virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr 
qemuCaps,
 
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE))
 VIR_DOMAIN_CAPS_ENUM_SET(disk-bus, VIR_DOMAIN_DISK_BUS_USB);
+return 0;
 }
 
 
-static void
+static int
 virQEMUCapsFillDomainDeviceHostdevCaps(virQEMUCapsPtr qemuCaps,
virDomainCapsDeviceHostdevPtr hostdev)
 {
@@ -3715,10 +3719,11 @@ virQEMUCapsFillDomainDeviceHostdevCaps(virQEMUCapsPtr 
qemuCaps,
  VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT,
  VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM);
 }
+return 0;
 }
 
 
-void
+int
 virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
   virQEMUCapsPtr qemuCaps)
 {
@@ -3729,7 +3734,9 @@ virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
 
 domCaps-maxvcpus = maxvcpus;
 
-virQEMUCapsFillDomainOSCaps(qemuCaps, os, domCaps-arch);
-virQEMUCapsFillDomainDeviceDiskCaps(qemuCaps, disk);
-virQEMUCapsFillDomainDeviceHostdevCaps(qemuCaps, hostdev);
+if (virQEMUCapsFillDomainOSCaps(qemuCaps, os, domCaps-arch)  0 ||
+virQEMUCapsFillDomainDeviceDiskCaps(qemuCaps, disk)  0 ||
+virQEMUCapsFillDomainDeviceHostdevCaps(qemuCaps, hostdev)  0)
+return -1;
+return 0;
 }
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 0980c00..828bba3 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -324,7 +324,7 @@ int virQEMUCapsInitGuestFromBinary(virCapsPtr caps,
virQEMUCapsPtr kvmbinCaps,
virArch guestarch);
 
-void virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
-   virQEMUCapsPtr qemuCaps);
+int virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
+  virQEMUCapsPtr qemuCaps);
 
 #endif /* __QEMU_CAPABILITIES_H__*/
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 15ad64d..4fe5909 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17357,7 +17357,8 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn,
 if (!(domCaps = virDomainCapsNew(emulatorbin, machine, arch, virttype)))
 goto cleanup;
 
-virQEMUCapsFillDomainCaps(domCaps, qemuCaps);
+if (virQEMUCapsFillDomainCaps(domCaps, qemuCaps)  0)
+goto cleanup;
 
 ret = virDomainCapsFormat(domCaps);
  cleanup:
diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
index f240643..0c4b09f 100644
--- a/tests/domaincapstest.c
+++ b/tests/domaincapstest.c
@@ -28,13 +28,13 @@
 
 #define VIR_FROM_THIS VIR_FROM_NONE
 
-typedef void (*virDomainCapsFill)(virDomainCapsPtr domCaps,
-  void *opaque);
+typedef int (*virDomainCapsFill)(virDomainCapsPtr domCaps,
+ void *opaque);
 
 #define SET_ALL_BITS(x) \
 memset((x.values), 0xff, sizeof(x.values))
 
-static void
+static int
 fillAll(virDomainCapsPtr domCaps,
 void *opaque ATTRIBUTE_UNUSED)
 {
@@ -60,18 +60,20 @@ fillAll(virDomainCapsPtr domCaps,

[libvirt] [PATCH v2 2/2] domaincaps: Expose UEFI binary path, if it exists

2014-09-17 Thread Michal Privoznik
From: Cole Robinson crobi...@redhat.com

Check to see if the UEFI binary mentioned in qemu.conf actually
exists, and if so expose it in domcapabilities like

loader ...
  value/path/to/ovmf/value
/loader

We introduce some generic domcaps infrastructure for handling
a dynamic list of string values, it may be of use for future bits.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 docs/formatdomaincaps.html.in  |  6 +++
 docs/schemas/domaincaps.rng| 17 +---
 src/conf/domain_capabilities.c | 29 +
 src/conf/domain_capabilities.h |  8 
 src/qemu/qemu_capabilities.c   | 32 +++---
 src/qemu/qemu_capabilities.h   |  7 +++-
 src/qemu/qemu_driver.c |  6 ++-
 tests/domaincapsschemadata/domaincaps-full.xml |  2 +
 .../domaincaps-qemu_1.6.50-1.xml   |  1 +
 tests/domaincapstest.c | 49 +++---
 10 files changed, 140 insertions(+), 17 deletions(-)

diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
index 34d746d..6959dfe 100644
--- a/docs/formatdomaincaps.html.in
+++ b/docs/formatdomaincaps.html.in
@@ -105,6 +105,7 @@
   ...
   lt;os supported='yes'gt;
 lt;loader supported='yes'gt;
+  lt;valuegt;/usr/share/OVMF/OVMF_CODE.fdlt;/valuegt;
   lt;enum name='type'gt;
 lt;valuegt;romlt;/valuegt;
 lt;valuegt;pflashlt;/valuegt;
@@ -122,6 +123,11 @@
 pFor the codeloader/code element, the following can occur:/p
 
 dl
+  dtvalue/dt
+  ddList of known loader paths. Currently this is only used
+  to advertise known locations of OVMF binaries for qemu. Binaries
+  will only be listed if they actually exist on disk./dd
+
   dttype/dt
   ddWhether loader is a typical BIOS (coderom/code) or
   an UEFI binary (codepflash/code). This refers to
diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
index ad8d966..f4a555f 100644
--- a/docs/schemas/domaincaps.rng
+++ b/docs/schemas/domaincaps.rng
@@ -47,6 +47,9 @@
   define name='loader'
 element name='loader'
   ref name='supported'/
+  optional
+ref name='value'/
+  /optional
   ref name='enum'/
 /element
   /define
@@ -85,6 +88,14 @@
 /element
   /define
 
+  define name='value'
+zeroOrMore
+  element name='value'
+text/
+  /element
+/zeroOrMore
+  /define
+
   define name='supported'
 attribute name='supported'
   choice
@@ -100,11 +111,7 @@
 attribute name='name'
   text/
 /attribute
-zeroOrMore
-  element name='value'
-text/
-  /element
-/zeroOrMore
+ref name='value'/
   /element
 /zeroOrMore
   /define
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 5a3c8e7..7c59912 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -48,12 +48,28 @@ VIR_ONCE_GLOBAL_INIT(virDomainCaps)
 
 
 static void
+virDomainCapsStringValuesFree(virDomainCapsStringValuesPtr values)
+{
+size_t i;
+
+if (!values || !values-values)
+return;
+
+for (i = 0; i  values-nvalues; i++)
+VIR_FREE(values-values[i]);
+VIR_FREE(values-values);
+}
+
+
+static void
 virDomainCapsDispose(void *obj)
 {
 virDomainCapsPtr caps = obj;
 
 VIR_FREE(caps-path);
 VIR_FREE(caps-machine);
+
+virDomainCapsStringValuesFree(caps-os.loader.values);
 }
 
 
@@ -156,6 +172,18 @@ virDomainCapsEnumFormat(virBufferPtr buf,
 return ret;
 }
 
+
+static void
+virDomainCapsStringValuesFormat(virBufferPtr buf,
+virDomainCapsStringValuesPtr values)
+{
+size_t i;
+
+for (i = 0; i  values-nvalues; i++)
+virBufferEscapeString(buf, value%s/value\n, values-values[i]);
+}
+
+
 #define FORMAT_PROLOGUE(item)   \
 do {\
 virBufferAsprintf(buf,  #item  supported='%s'%s\n, \
@@ -185,6 +213,7 @@ virDomainCapsLoaderFormat(virBufferPtr buf,
 {
 FORMAT_PROLOGUE(loader);
 
+virDomainCapsStringValuesFormat(buf, loader-values);
 ENUM_PROCESS(loader, type, virDomainLoaderTypeToString);
 ENUM_PROCESS(loader, readonly, virTristateBoolTypeToString);
 
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index 768646b..597ac75 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -37,6 +37,13 @@ struct _virDomainCapsEnum {
 unsigned int values; /* Bitmask of values supported in the corresponding 
enum */
 };
 
+typedef struct _virDomainCapsStringValues virDomainCapsStringValues;
+typedef virDomainCapsStringValues *virDomainCapsStringValuesPtr;
+struct _virDomainCapsStringValues {
+char **values;  /* raw string values */
+

Re: [libvirt] [PATCH v2 1/2] qemu_capabilities: Change virQEMUCapsFillDomainCaps signature

2014-09-17 Thread Laszlo Ersek
On 09/17/14 14:15, Michal Privoznik wrote:
 Up till now the virQEMUCapsFillDomainCaps() was type of void as
 there was no way for it to fail. This is, however, going to
 change in the next commit.
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  src/qemu/qemu_capabilities.c | 25 -
  src/qemu/qemu_capabilities.h |  4 ++--
  src/qemu/qemu_driver.c   |  3 ++-
  tests/domaincapstest.c   | 19 ---
  4 files changed, 32 insertions(+), 19 deletions(-)

Seems reasonable.

Acked-by: Laszlo Ersek ler...@redhat.com

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


Re: [libvirt] [PATCH v2 2/2] domaincaps: Expose UEFI binary path, if it exists

2014-09-17 Thread Laszlo Ersek
On 09/17/14 14:15, Michal Privoznik wrote:

 diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
 index 0c4b09f..8543963 100644
 --- a/tests/domaincapstest.c
 +++ b/tests/domaincapstest.c
 @@ -34,6 +34,27 @@ typedef int (*virDomainCapsFill)(virDomainCapsPtr domCaps,
  #define SET_ALL_BITS(x) \
  memset((x.values), 0xff, sizeof(x.values))
  
 +static int ATTRIBUTE_SENTINEL
 +fillStringValues(virDomainCapsStringValuesPtr values, ...)
 +{
 +int ret = 0;
 +va_list list;
 +const char *str;
 +
 +va_start(list, values);
 +while ((str = va_arg(list, const char *))) {
 +if (VIR_REALLOC_N(values-values, values-nvalues + 1)  0 ||
 +VIR_STRDUP(values-values[values-nvalues], str)  0) {
 +ret = -1;
 +break;
 +}
 +values-nvalues++;
 +}
 +va_end(list);
 +
 +return ret;
 +}

Okay, you increment values-nvalues only after.

The rest too looks good to me.

Acked-by: Laszlo Ersek ler...@redhat.com

Thanks
Laszlo

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


Re: [libvirt] [PATCH v2 1/2] qemu_capabilities: Change virQEMUCapsFillDomainCaps signature

2014-09-17 Thread Cole Robinson
On 09/17/2014 08:15 AM, Michal Privoznik wrote:
 Up till now the virQEMUCapsFillDomainCaps() was type of void as
 there was no way for it to fail. This is, however, going to
 change in the next commit.
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  src/qemu/qemu_capabilities.c | 25 -
  src/qemu/qemu_capabilities.h |  4 ++--
  src/qemu/qemu_driver.c   |  3 ++-
  tests/domaincapstest.c   | 19 ---
  4 files changed, 32 insertions(+), 19 deletions(-)

ACK

- Cole

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


Re: [libvirt] [PATCH v2 2/2] domaincaps: Expose UEFI binary path, if it exists

2014-09-17 Thread Cole Robinson
On 09/17/2014 08:15 AM, Michal Privoznik wrote:
 From: Cole Robinson crobi...@redhat.com
 
 Check to see if the UEFI binary mentioned in qemu.conf actually
 exists, and if so expose it in domcapabilities like
 
 loader ...
   value/path/to/ovmf/value
 /loader
 
 We introduce some generic domcaps infrastructure for handling
 a dynamic list of string values, it may be of use for future bits.
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com


ACK, thanks for cleaning it up. But please change authorship since the patch
is reasonably altered, I won't be offended :)

- Cole

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


Re: [libvirt] [PATCH v2] rpc: make daemon spawning a bit more intelligent

2014-09-17 Thread Martin Kletzander

On Tue, Sep 16, 2014 at 10:22:53AM -0400, John Ferlan wrote:



On 09/16/2014 05:16 AM, Martin Kletzander wrote:

This way it behaves more like the daemon itself does (acquiring a
pidfile, deleting the socket before binding, etc.).

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=927369
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1138604

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/rpc/virnetsocket.c | 65 +++---
 1 file changed, 57 insertions(+), 8 deletions(-)



The error path/retry logic needs a tweak... I added some inline thinking
since we don't have a virtual whiteboard to share on this!



Yes, it does.  I didn't think it through from scratch, just adjusted
to your comments.  This time I went through it few times.  Just let me
confirm I understood what you meant everywhere.


diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 80aeddf..7be1492 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -51,9 +51,11 @@
 #include virlog.h
 #include virfile.h
 #include virthread.h
+#include virpidfile.h
 #include virprobe.h
 #include virprocess.h
 #include virstring.h
+#include dirname.h
 #include passfd.h

 #if WITH_SSH2
@@ -541,7 +543,10 @@ int virNetSocketNewConnectUNIX(const char *path,
const char *binary,
virNetSocketPtr *retsock)
 {
+char *binname = NULL;
+char *pidpath = NULL;
 int fd, passfd = -1;
+int pidfd = -1;
 virSocketAddr localAddr;
 virSocketAddr remoteAddr;

@@ -580,16 +585,47 @@ int virNetSocketNewConnectUNIX(const char *path,
 goto error;
 }

+if (!(binname = last_component(binary)) || binname[0] == '\0') {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _(Cannot determine basename for binary '%s'),
+   binary);
+goto error;
+}
+
+if (virPidFileConstructPath(false, NULL, binname, pidpath)  0)
+goto error;


Since the first param is false, we are guaranteed only that 'pidpath' is
the path to the virGetUserRuntimeDirectory() for $binname.pid.

We are not sure if we created the path in virFileMakePathHelper() or
not. If we later then delete the file on the error path how does that
affect the daemon that wins the race?  See the conundrum?



This is only about deleting the pidfile, right?  Deleting it only when
it is acquired (pidfd = 0) should fix this.

I'll try describing it here a little bit more:

virNetSocketNewConnectUNIX() is called with (spawnDaemon == true) only
if (privileged == false).  virPidFileConstructPath() is called also
only when (spawnDaemon == true) and guarantees that the path for the
pidfile exists and is constructed the same way it is in daemon.  The
path should not be deleted no matter whether we fail or not, those
directories should be kept there for later.


+
+pidfd = virPidFileAcquirePath(pidpath, false, getpid());
+VIR_FREE(pidpath);


Because you VIR_FREE() here, there is no way for the error: path to have
a non NULL pidpath... and delete the pidpath.



Using VIR_FREE(pidpath); in both error path and before return 0 (my
initial idea) should take care of this.


+if (pidfd  0) {


Getting here means we were unable to get the pidfile lock and I don't
think we want to delete the pidpath... Since it's probably owned by some
other daemon



if (pidfd  0) then it means it will not get deleted.  By the way it
doesn't have to be deleted, closing should be enough to unlock the
file.


+/*
+ * This can happen in a very rare case of two clients spawning two
+ * daemons at the same time, and the error in the logs that gets
+ * reset here can be a clue to some future debugging.
+ */
+virResetLastError();
+spawnDaemon = false;
+goto retry;
+}


If we get here, we've written our pid into the pidfile and we have have
the lock... So that means we should own the file. Errors from here
should delete the file.



Right, there's nothing wrong.


+
 if ((passfd = socket(PF_UNIX, SOCK_STREAM, 0))  0) {
 virReportSystemError(errno, %s, _(Failed to create socket));
 goto error;
 }

 /*
- * We have to fork() here, because umask() is set
- * per-process, chmod() is racy and fchmod() has undefined
- * behaviour on sockets according to POSIX, so it doesn't
- * work outside Linux.
+ * We already even acquired the pidfile, so no one else should be using
+ * @path right now.  So we're OK to unlink it and paying attention to
+ * the return value makes no real sense here.  Only if it's not an
+ * abstract socket, of course.
+ */
+if (path[0] != '@')
+unlink(path);
+
+/*
+ * We have to fork() here, 

Re: [libvirt] [PATCH v2 2/2] domaincaps: Expose UEFI binary path, if it exists

2014-09-17 Thread Michal Privoznik

On 17.09.2014 14:57, Cole Robinson wrote:

On 09/17/2014 08:15 AM, Michal Privoznik wrote:

From: Cole Robinson crobi...@redhat.com

Check to see if the UEFI binary mentioned in qemu.conf actually
exists, and if so expose it in domcapabilities like

loader ...
   value/path/to/ovmf/value
/loader

We introduce some generic domcaps infrastructure for handling
a dynamic list of string values, it may be of use for future bits.

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



ACK, thanks for cleaning it up. But please change authorship since the patch
is reasonably altered, I won't be offended :)

- Cole



Thanks. Fixed and pushed.

Michal

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


[libvirt] [PATCH 2/n] dumpxml: prepare to output block info

2014-09-17 Thread Eric Blake
This patch adds the common code for outputting drive sizing
information, when a user has requested the new API flag from
the previous patch.

* src/util/virstoragefile.h (_virStorageSource): Add physical, to
mirror virDomainBlockInfo.
* docs/schemas/domaincommon.rng (storageSourceExtra): New define.
* src/conf/domain_conf.c (virDomainDiskDefFormat): Output sizing
when flag is set.
(DUMPXML_FLAGS): Add new flag.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 docs/schemas/domaincommon.rng | 22 ++
 src/conf/domain_conf.c| 16 +++-
 src/util/virstoragefile.h |  3 ++-
 3 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index c600f22..dd874fc 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1292,6 +1292,28 @@
 /element
   /define

+  define name='storageSourceExtra' combine='choice'
+!-- Override of storagecommon.rng to allow domain disks to list
+ extras without impacting snapshot disks --
+interleave
+  optional
+element name=capacity
+  ref name=scaledInteger/
+/element
+  /optional
+  optional
+element name=allocation
+  ref name=scaledInteger/
+/element
+  /optional
+  optional
+element name=physical
+  ref name=scaledInteger/
+/element
+  /optional
+/interleave
+  /define
+
   define name=diskBackingChain
 choice
   ref name=diskBackingStore/
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3ccec1c..3b54619 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -108,7 +108,8 @@ typedef enum {
 (VIR_DOMAIN_XML_SECURE |\
  VIR_DOMAIN_XML_INACTIVE |  \
  VIR_DOMAIN_XML_UPDATE_CPU |\
- VIR_DOMAIN_XML_MIGRATABLE)
+ VIR_DOMAIN_XML_MIGRATABLE |   \
+ VIR_DOMAIN_XML_BLOCK_INFO)

 verify(((VIR_DOMAIN_XML_INTERNAL_STATUS |
  VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET |
@@ -15887,6 +15888,19 @@ virDomainDiskDefFormat(virBufferPtr buf,
   flags)  0)
 return -1;

+if (flags  VIR_DOMAIN_XML_BLOCK_INFO) {
+if (def-src-capacity)
+virBufferAsprintf(buf, capacity unit='bytes'%llu/capacity\n,
+  def-src-capacity);
+if (def-src-allocation)
+virBufferAsprintf(buf,
+  allocation unit='bytes'%llu/allocation\n,
+  def-src-allocation);
+if (def-src-physical)
+virBufferAsprintf(buf, physical unit='bytes'%llu/physical\n,
+  def-src-physical);
+}
+
 /* Don't format backingStore to inactive XMLs until the code for
  * persistent storage of backing chains is ready. */
 if (!(flags  VIR_DOMAIN_XML_INACTIVE) 
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 2583e10..681e50a 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -252,8 +252,9 @@ struct _virStorageSource {

 virStoragePermsPtr perms;
 virStorageTimestampsPtr timestamps;
-unsigned long long allocation; /* in bytes, 0 if unknown */
 unsigned long long capacity; /* in bytes, 0 if unknown */
+unsigned long long allocation; /* in bytes, 0 if unknown */
+unsigned long long physical; /* in bytes, 0 if unknown */
 size_t nseclabels;
 virSecurityDeviceLabelDefPtr *seclabels;

-- 
1.9.3

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


[libvirt] [RFC: PATCH 0/2] Display allocation during dumpxml

2014-09-17 Thread Eric Blake
I'm still working on code to populate the latest numbers for
each disk of a domain, including getting numbers for offline
domains, but have confirmed that with these two patches alone
I'm able to see capacity and allocation numbers for block
volumes of live domains (thanks to how we populate backing
chain information).  So while there are more patches to come,
I'd like to get review started on my proposed API addition.

Eric Blake (2):
  dumpxml: add flag to virDomainGetXMLDesc
  dumpxml: prepare to output block info

 docs/schemas/domaincommon.rng | 22 ++
 include/libvirt/libvirt.h.in  |  1 +
 src/conf/domain_conf.c| 16 +++-
 src/libvirt.c | 15 +++
 src/util/virstoragefile.h |  3 ++-
 tools/virsh-domain.c  |  6 ++
 tools/virsh.pod   |  6 --
 7 files changed, 61 insertions(+), 8 deletions(-)

-- 
1.9.3

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


[libvirt] [PATCH 1/n] dumpxml: add flag to virDomainGetXMLDesc

2014-09-17 Thread Eric Blake
The information in virDomainGetBlockInfo() is important for clients
that use qcow2 format on LVM block devices - by tracking the
allocation in relation to the physical size, management can tell
if a disk needs to be expanded before the guest (file system
contents) and/or qemu (copy-on-write differing more from a backing
file) runs out of space.  Normally, only the active layer matters,
but during a block commit operation, the allocation of the backing
file ALSO grows, and management would like to track that growth.

Right now, virDomainGetBlockInfo() can only convey information
about the active layer of a disk, via a single API call per disk.
It can also be easily extended to support vda[1] notation that
we recently added for blockcommit and friends, to get similar
information about a backing element; but that still implies one
call per layer, which adds up to a lot of overhead.

This API addition will make it possible to grab this information
for ALL guest disks in a single call, by letting the user request
additional information about each disk in the backing chain to be
output as part of the domain XML.  My ultimate goal is to have
this flag and virStorageVolGetXMLDesc() expose the same bits of
information about a given storage volume (there are slight
incompatiblities in the XML naming that we'll have to keep for
back-compat sake, but the idea is to get all the information out
there).

* include/libvirt/libvirt.h.in (virDomainXMLFlags): Add new flag.
* src/libvirt.c (virDomainGetXMLDesc): Document it.
(virDomainSaveImageGetXMLDesc, virDomainSnapshotGetXMLDesc): For
now, mention the flag won't help here.
* tools/virsh-domain.c (cmdDumpXML): Add new flag.
* tools/virsh.pod (dumpxml): Document it.

Signed-off-by: Eric Blake ebl...@redhat.com
---
 include/libvirt/libvirt.h.in |  1 +
 src/libvirt.c| 15 +++
 tools/virsh-domain.c |  6 ++
 tools/virsh.pod  |  6 --
 4 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index c2f9d26..40f4e0c 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2082,6 +2082,7 @@ typedef enum {
 VIR_DOMAIN_XML_INACTIVE = (1  1), /* dump inactive domain 
information */
 VIR_DOMAIN_XML_UPDATE_CPU   = (1  2), /* update guest CPU requirements 
according to host CPU */
 VIR_DOMAIN_XML_MIGRATABLE   = (1  3), /* dump XML suitable for migration 
*/
+VIR_DOMAIN_XML_BLOCK_INFO   = (1  4), /* include storage volume 
information about each disk source */
 } virDomainXMLFlags;

 char *  virDomainGetXMLDesc (virDomainPtr domain,
diff --git a/src/libvirt.c b/src/libvirt.c
index f7e5a37..1020cc5 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -2806,8 +2806,9 @@ virDomainRestoreFlags(virConnectPtr conn, const char 
*from, const char *dxml,
  *
  * No security-sensitive data will be included unless @flags contains
  * VIR_DOMAIN_XML_SECURE; this flag is rejected on read-only
- * connections.  For this API, @flags should not contain either
- * VIR_DOMAIN_XML_INACTIVE or VIR_DOMAIN_XML_UPDATE_CPU.
+ * connections.  For this API, @flags should not contain
+ * VIR_DOMAIN_XML_INACTIVE, VIR_DOMAIN_XML_UPDATE_CPU, or
+ * VIR_DOMAIN_XML_BLOCK_INFO.
  *
  * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of
  * error.  The caller must free() the returned value.
@@ -4354,6 +4355,11 @@ virDomainGetControlInfo(virDomainPtr domain,
  * describing CPU capabilities is modified to match actual
  * capabilities of the host.
  *
+ * If @flags contains VIR_DOMAIN_XML_BLOCK_INFO, the listing for each
+ * disk device will contain additional information such as capacity
+ * and allocation, similar to what is displayed by virStorageVolGetXMLDesc(),
+ * and avoiding the need to call virDomainGetBlockInfo() separately.
+ *
  * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of error.
  * the caller must free() the returned value.
  */
@@ -18507,8 +18513,9 @@ virDomainSnapshotCreateXML(virDomainPtr domain,
  *
  * No security-sensitive data will be included unless @flags contains
  * VIR_DOMAIN_XML_SECURE; this flag is rejected on read-only
- * connections.  For this API, @flags should not contain either
- * VIR_DOMAIN_XML_INACTIVE or VIR_DOMAIN_XML_UPDATE_CPU.
+ * connections.  For this API, @flags should not contain
+ * VIR_DOMAIN_XML_INACTIVE, VIR_DOMAIN_XML_UPDATE_CPU, or
+ * VIR_DOMAIN_XML_BLOCK_INFO.
  *
  * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of error.
  * the caller must free() the returned value.
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 435d045..8f97c48 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -8838,6 +8838,10 @@ static const vshCmdOptDef opts_dumpxml[] = {
  .type = VSH_OT_BOOL,
  .help = N_(provide XML suitable for migrations)
 },
+{.name = block-info,
+ .type = 

[libvirt] [PATCH 2/6] qemu: Fix old tcp:host URIs more cleanly

2014-09-17 Thread Jiri Denemark
For compatibility with old libvirt we need to support both tcp:host and
tcp://host migration URIs. Let's make the code that parses them a bit
cleaner.

Signed-off-by: Jiri Denemark jdene...@redhat.com
---

Notes:
Version 3:
- new patch to make qemu: RDMA migration support a bit more
  straightforward

 src/qemu/qemu_migration.c | 77 ---
 1 file changed, 33 insertions(+), 44 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index d738f9b..5741de2 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2817,6 +2817,29 @@ qemuMigrationPrepareTunnel(virQEMUDriverPtr driver,
 }
 
 
+static virURIPtr
+qemuMigrationParseURI(const char *uri, bool *wellFormed)
+{
+char *tmp = NULL;
+virURIPtr parsed;
+
+/* For compatibility reasons tcp://... URIs are sent as tcp:...
+ * We need to transform them to a well-formed URI before parsing. */
+if (STRPREFIX(uri, tcp:)  !STRPREFIX(uri + 4, //)) {
+if (virAsprintf(tmp, tcp://%s, uri + 4)  0)
+return NULL;
+uri = tmp;
+}
+
+parsed = virURIParse(uri);
+if (parsed  wellFormed)
+*wellFormed = !tmp;
+VIR_FREE(tmp);
+
+return parsed;
+}
+
+
 int
 qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
virConnectPtr dconn,
@@ -2834,11 +2857,8 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
 unsigned short port = 0;
 bool autoPort = true;
 char *hostname = NULL;
-const char *p;
-char *uri_str = NULL;
 int ret = -1;
 virURIPtr uri = NULL;
-bool well_formed_uri = true;
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 const char *migrateHost = cfg-migrateHost;
 
@@ -2890,34 +2910,18 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
  * compatibility with old targets. We at least make the
  * new targets accept both syntaxes though.
  */
-/* Caller frees */
 if (virAsprintf(uri_out, tcp:%s:%d, hostname, port)  0)
 goto cleanup;
 } else {
-/* Check the URI starts with tcp:.  We will escape the
- * URI when passing it to the qemu monitor, so bad
- * characters in hostname part don't matter.
- */
-if (!(p = STRSKIP(uri_in, tcp:))) {
-virReportError(VIR_ERR_INVALID_ARG, %s,
-   _(only tcp URIs are supported for KVM/QEMU
-  migrations));
+bool well_formed_uri;
+
+if (!(uri = qemuMigrationParseURI(uri_in, well_formed_uri)))
 goto cleanup;
-}
 
-/* Convert uri_in to well-formed URI with // after tcp: */
-if (!(STRPREFIX(uri_in, tcp://))) {
-well_formed_uri = false;
-if (virAsprintf(uri_str, tcp://%s, p)  0)
-goto cleanup;
-}
-
-uri = virURIParse(uri_str ? uri_str : uri_in);
-VIR_FREE(uri_str);
-
-if (uri == NULL) {
-virReportError(VIR_ERR_INVALID_ARG, _(unable to parse URI: %s),
-   uri_in);
+if (STRNEQ(uri-scheme, tcp)) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+   _(unsupported scheme %s in migration URI %s),
+   uri-scheme, uri_in);
 goto cleanup;
 }
 
@@ -2931,27 +2935,22 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
 if (virPortAllocatorAcquire(driver-migrationPorts, port)  0)
 goto cleanup;
 
+/* Send well-formed URI only if uri_in was well-formed */
 if (well_formed_uri) {
 uri-port = port;
-
-/* Caller frees */
 if (!(*uri_out = virURIFormat(uri)))
 goto cleanup;
 } else {
-/* Caller frees */
 if (virAsprintf(uri_out, %s:%d, uri_in, port)  0)
 goto cleanup;
 }
-
 } else {
 port = uri-port;
 autoPort = false;
 }
 }
 
-if (*uri_out)
-VIR_DEBUG(Generated uri_out=%s, *uri_out);
-
+VIR_DEBUG(Generated uri_out=%s, *uri_out);
 ret = qemuMigrationPrepareAny(driver, dconn, cookiein, cookieinlen,
   cookieout, cookieoutlen, def, origname,
   NULL, port, autoPort, listenAddress, flags);
@@ -3704,17 +3703,7 @@ static int doNativeMigrate(virQEMUDriverPtr driver,
   cookieout, cookieoutlen, flags, resource,
   NULLSTR(graphicsuri));
 
-if (STRPREFIX(uri, tcp:)  !STRPREFIX(uri, tcp://)) {
-char *tmp;
-/* HACK: source host generates bogus URIs, so fix them up */
-if (virAsprintf(tmp, tcp://%s, uri + strlen(tcp:))  0)
-return -1;
-uribits = virURIParse(tmp);
-VIR_FREE(tmp);
-} else {
-uribits = virURIParse(uri);

[libvirt] [PATCH 0/6] RDMA migration support

2014-09-17 Thread Jiri Denemark
This is a modified version of RDMA migration patches sent back in
January by Michael R. Hines. See individual patches for (numerous)
changes since v2.

Jiri Denemark (3):
  qemu: Fix old tcp:host URIs more cleanly
  qemu: Prepare support for arbitrary migration protocol
  qemu: Add RDMA migration capabilities

Michael R. Hines (3):
  qemu: Expose additional migration statistics
  qemu: RDMA migration support
  qemu: Memory pre-pinning support for RDMA migration

 include/libvirt/libvirt.h.in |   26 +
 src/qemu/qemu.conf   |8 +
 src/qemu/qemu_capabilities.c |   32 +-
 src/qemu/qemu_capabilities.h |1 +
 src/qemu/qemu_command.c  |8 +
 src/qemu/qemu_domain.c   |   18 +
 src/qemu/qemu_migration.c|  216 +-
 src/qemu/qemu_migration.h|3 +-
 src/qemu/qemu_monitor.c  |   25 +-
 src/qemu/qemu_monitor.h  |   13 +
 src/qemu/qemu_monitor_json.c |   61 +-
 src/qemu/qemu_monitor_json.h |2 +
 tests/qemucapabilitiesdata/caps_1.2.2-1.replies  |   10 +
 tests/qemucapabilitiesdata/caps_1.3.1-1.replies  |   10 +
 tests/qemucapabilitiesdata/caps_1.4.2-1.replies  |   10 +
 tests/qemucapabilitiesdata/caps_1.5.3-1.replies  |   10 +
 tests/qemucapabilitiesdata/caps_1.6.0-1.replies  |   22 +
 tests/qemucapabilitiesdata/caps_1.6.50-1.replies |   22 +
 tests/qemucapabilitiesdata/caps_2.1.1-1.caps |  162 ++
 tests/qemucapabilitiesdata/caps_2.1.1-1.replies  | 3264 ++
 tests/qemucapabilitiestest.c |1 +
 tools/virsh-domain.c |   34 +
 22 files changed, 3886 insertions(+), 72 deletions(-)
 create mode 100644 tests/qemucapabilitiesdata/caps_2.1.1-1.caps
 create mode 100644 tests/qemucapabilitiesdata/caps_2.1.1-1.replies

-- 
2.1.0

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


[libvirt] [PATCH 6/6] qemu: Memory pre-pinning support for RDMA migration

2014-09-17 Thread Jiri Denemark
From: Michael R. Hines mrhi...@us.ibm.com

RDMA Live migration requires registering memory with the hardware, and
thus QEMU offers a new 'capability' to pre-register / mlock() the guest
memory in advance for higher RDMA performance before the migration
begins. This capability is disabled by default, which means QEMU will
register the memory with the hardware in an on-demand basis.

This patch exposes this capability with the following example usage:

virsh migrate --live --rdma-pin-all --migrateuri rdma://hostname domain 
qemu+ssh://hostname/system

Signed-off-by: Michael R. Hines mrhi...@us.ibm.com
Signed-off-by: Jiri Denemark jdene...@redhat.com
---

Notes:
Version 3:
- moved rdma-pin-all capability into qemu: Add RDMA migration
  capabilities patch
- removed magic computation of mlock memory limit

 include/libvirt/libvirt.h.in |  1 +
 src/qemu/qemu_migration.c| 49 
 src/qemu/qemu_migration.h|  3 ++-
 tools/virsh-domain.c |  7 +++
 4 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 702f797..a028e2d 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -1224,6 +1224,7 @@ typedef enum {
 VIR_MIGRATE_COMPRESSED= (1  11), /* compress data during 
migration */
 VIR_MIGRATE_ABORT_ON_ERROR= (1  12), /* abort migration on I/O 
errors happened during migration */
 VIR_MIGRATE_AUTO_CONVERGE = (1  13), /* force convergence */
+VIR_MIGRATE_RDMA_PIN_ALL  = (1  14), /* RDMA memory pinning */
 } virDomainMigrateFlags;
 
 
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index b59e94d..2daac48 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1874,6 +1874,46 @@ qemuMigrationSetAutoConverge(virQEMUDriverPtr driver,
 
 
 static int
+qemuMigrationSetPinAll(virQEMUDriverPtr driver,
+   virDomainObjPtr vm,
+   qemuDomainAsyncJob job)
+{
+qemuDomainObjPrivatePtr priv = vm-privateData;
+int ret;
+
+if (qemuDomainObjEnterMonitorAsync(driver, vm, job)  0)
+return -1;
+
+ret = qemuMonitorGetMigrationCapability(
+priv-mon,
+QEMU_MONITOR_MIGRATION_CAPS_RDMA_PIN_ALL);
+
+if (ret  0) {
+goto cleanup;
+} else if (ret == 0) {
+if (job == QEMU_ASYNC_JOB_MIGRATION_IN) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
+   _(rdma pinning migration is not supported by 
+ target QEMU binary));
+} else {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
+   _(rdma pinning migration is not supported by 
+ source QEMU binary));
+}
+ret = -1;
+goto cleanup;
+}
+
+ret = qemuMonitorSetMigrationCapability(
+priv-mon,
+QEMU_MONITOR_MIGRATION_CAPS_RDMA_PIN_ALL);
+
+ cleanup:
+qemuDomainObjExitMonitor(driver, vm);
+return ret;
+}
+
+static int
 qemuMigrationWaitForSpice(virQEMUDriverPtr driver,
   virDomainObjPtr vm)
 {
@@ -2709,6 +2749,10 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 goto stop;
 }
 
+if (flags  VIR_MIGRATE_RDMA_PIN_ALL 
+qemuMigrationSetPinAll(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN)  0)
+goto stop;
+
 if (mig-lockState) {
 VIR_DEBUG(Received lockstate %s, mig-lockState);
 VIR_FREE(priv-lockState);
@@ -3530,6 +3574,11 @@ qemuMigrationRun(virQEMUDriverPtr driver,
  QEMU_ASYNC_JOB_MIGRATION_OUT)  0)
 goto cleanup;
 
+if (flags  VIR_MIGRATE_RDMA_PIN_ALL 
+qemuMigrationSetPinAll(driver, vm,
+   QEMU_ASYNC_JOB_MIGRATION_OUT)  0)
+goto cleanup;
+
 if (qemuDomainObjEnterMonitorAsync(driver, vm,
QEMU_ASYNC_JOB_MIGRATION_OUT)  0)
 goto cleanup;
diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h
index 3fa68dc..e7a90c3 100644
--- a/src/qemu/qemu_migration.h
+++ b/src/qemu/qemu_migration.h
@@ -40,7 +40,8 @@
  VIR_MIGRATE_OFFLINE |  \
  VIR_MIGRATE_COMPRESSED |   \
  VIR_MIGRATE_ABORT_ON_ERROR |   \
- VIR_MIGRATE_AUTO_CONVERGE)
+ VIR_MIGRATE_AUTO_CONVERGE |\
+ VIR_MIGRATE_RDMA_PIN_ALL)
 
 /* All supported migration parameters and their types. */
 # define QEMU_MIGRATION_PARAMETERS  \
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 105b99e..a6ced5f 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9212,6 +9212,10 @@ static const vshCmdOptDef opts_migrate[] = {
  .type = VSH_OT_BOOL,
  .help = N_(force convergence during live migration)
 },
+{.name = rdma-pin-all,
+   

[libvirt] [PATCH 3/6] qemu: Prepare support for arbitrary migration protocol

2014-09-17 Thread Jiri Denemark
Currently we only support TCP protocol for native QEMU migration but
this is going to be changed. Let's make the code more general and remove
hardcoded TCP protocol from several places.

Signed-off-by: Jiri Denemark jdene...@redhat.com
---

Notes:
Version 3:
- separated from qemu: RDMA migration support

 src/qemu/qemu_migration.c | 36 
 src/qemu/qemu_monitor.c   |  3 ++-
 src/qemu/qemu_monitor.h   |  1 +
 3 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 5741de2..d0e2653 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2457,6 +2457,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 virDomainDefPtr *def,
 const char *origname,
 virStreamPtr st,
+const char *protocol,
 unsigned short port,
 bool autoPort,
 const char *listenAddress,
@@ -2569,6 +2570,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 struct addrinfo *info = NULL;
 struct addrinfo hints = { .ai_flags = AI_ADDRCONFIG,
   .ai_socktype = SOCK_STREAM };
+const char *incFormat;
 
 if (getaddrinfo(::, NULL, hints, info) == 0) {
 freeaddrinfo(info);
@@ -2605,21 +2607,27 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 } else {
 /* listenAddress is a hostname */
 }
-} else {
+} else if (qemuIPv6Capable  hostIPv6Capable) {
 /* Listen on :: instead of 0.0.0.0 if QEMU understands it
  * and there is at least one IPv6 address configured
  */
-listenAddress = qemuIPv6Capable  hostIPv6Capable ?
-encloseAddress = true, :: : 0.0.0.0;
+listenAddress = ::;
+encloseAddress = true;
+} else {
+listenAddress = 0.0.0.0;
 }
 
-/* QEMU will be started with -incoming [IPv6 addr]:port,
- * -incoming IPv4 addr:port or -incoming hostname:port
+/* QEMU will be started with
+ *   -incoming protocol:[IPv6 addr]:port,
+ *   -incoming protocol:IPv4 addr:port, or
+ *   -incoming protocol:hostname:port
  */
-if ((encloseAddress 
- virAsprintf(migrateFrom, tcp:[%s]:%d, listenAddress, port)  
0) ||
-(!encloseAddress 
- virAsprintf(migrateFrom, tcp:%s:%d, listenAddress, port)  0))
+if (encloseAddress)
+incFormat = %s:[%s]:%d;
+else
+incFormat = %s:%s:%d;
+if (virAsprintf(migrateFrom, incFormat,
+protocol, listenAddress, port)  0)
 goto cleanup;
 }
 
@@ -2812,7 +2820,7 @@ qemuMigrationPrepareTunnel(virQEMUDriverPtr driver,
 
 ret = qemuMigrationPrepareAny(driver, dconn, cookiein, cookieinlen,
   cookieout, cookieoutlen, def, origname,
-  st, 0, false, NULL, flags);
+  st, NULL, 0, false, NULL, flags);
 return ret;
 }
 
@@ -2953,7 +2961,8 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
 VIR_DEBUG(Generated uri_out=%s, *uri_out);
 ret = qemuMigrationPrepareAny(driver, dconn, cookiein, cookieinlen,
   cookieout, cookieoutlen, def, origname,
-  NULL, port, autoPort, listenAddress, flags);
+  NULL, uri ? uri-scheme : tcp,
+  port, autoPort, listenAddress, flags);
  cleanup:
 virURIFree(uri);
 VIR_FREE(hostname);
@@ -3169,6 +3178,7 @@ struct _qemuMigrationSpec {
 enum qemuMigrationDestinationType destType;
 union {
 struct {
+const char *protocol;
 const char *name;
 int port;
 } host;
@@ -3536,6 +3546,7 @@ qemuMigrationRun(virQEMUDriverPtr driver,
 switch (spec-destType) {
 case MIGRATION_DEST_HOST:
 ret = qemuMonitorMigrateToHost(priv-mon, migrate_flags,
+   spec-dest.host.protocol,
spec-dest.host.name,
spec-dest.host.port);
 break;
@@ -3676,7 +3687,7 @@ qemuMigrationRun(virQEMUDriverPtr driver,
 goto cleanup;
 }
 
-/* Perform migration using QEMU's native TCP migrate support,
+/* Perform migration using QEMU's native migrate support,
  * not encrypted obviously
  */
 static int doNativeMigrate(virQEMUDriverPtr driver,
@@ -3710,6 +3721,7 @@ static int doNativeMigrate(virQEMUDriverPtr driver,
 spec.destType = MIGRATION_DEST_CONNECT_HOST;
 else
 spec.destType = MIGRATION_DEST_HOST;
+spec.dest.host.protocol = uribits-scheme;
 spec.dest.host.name = uribits-server;
 

[libvirt] [PATCH 5/6] qemu: RDMA migration support

2014-09-17 Thread Jiri Denemark
From: Michael R. Hines mrhi...@us.ibm.com

This patch adds support for RDMA protocol in migration URIs.

USAGE: $ virsh migrate --live --migrateuri rdma://hostname domain 
qemu+ssh://hostname/system

Since libvirt runs QEMU in a pretty restricted environment, several
files needs to be added to cgroup_device_acl (in qemu.conf) for QEMU to
be able to access the host's infiniband hardware. Full documenation of
the feature can be found on QEMU wiki:
http://wiki.qemu.org/Features/RDMALiveMigration

Signed-off-by: Michael R. Hines mrhi...@us.ibm.com
Signed-off-by: Jiri Denemark jdene...@redhat.com
---

Notes:
The question is whether the IB devices should be added to
cgroup_device_acl by default or not...

Version 3:
- moved capabilities code into a separate patch
- got rid of migration URI hacks
- removed hacks that disabled IPv6 with RDMA
- moved refactoring into a dedicated patch
- documented IB devices which need to be added to cgroup acl in
  qemu.conf
- forbid RDMA migrations unless memory hard limit is set until we
  have a better plan for setting limits for mlock
- set QEMU's RLIMIT_MEMLOCK to memory hard_limit before starting
  RDMA migration
- check if RDMA migration is supported by source QEMU before trying
  to migrate

 src/qemu/qemu.conf|  8 
 src/qemu/qemu_command.c   |  8 
 src/qemu/qemu_migration.c | 39 +--
 3 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 79bba36..92ca715 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -274,6 +274,14 @@
 #/dev/ptmx, /dev/kvm, /dev/kqemu,
 #/dev/rtc,/dev/hpet, /dev/vfio/vfio
 #]
+#
+# RDMA migration requires the following extra files to be added to the list:
+#   /dev/infiniband/rdma_cm,
+#   /dev/infiniband/issm0,
+#   /dev/infiniband/issm1,
+#   /dev/infiniband/umad0,
+#   /dev/infiniband/umad1,
+#   /dev/infiniband/uverbs0
 
 
 # The default format for Qemu/KVM guest save images is raw; that is, the
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a892d99..fceed62 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9399,6 +9399,14 @@ qemuBuildCommandLine(virConnectPtr conn,
 goto error;
 }
 virCommandAddArg(cmd, migrateFrom);
+} else if (STRPREFIX(migrateFrom, rdma)) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_RDMA)) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+   _(incoming RDMA migration is not supported 
+ with this QEMU binary));
+goto error;
+}
+virCommandAddArg(cmd, migrateFrom);
 } else if (STREQ(migrateFrom, stdio)) {
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD)) {
 virCommandAddArgFormat(cmd, fd:%d, migrateFd);
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index d0e2653..b59e94d 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -56,6 +56,7 @@
 #include virhook.h
 #include virstring.h
 #include virtypedparam.h
+#include virprocess.h
 
 #define VIR_FROM_THIS VIR_FROM_QEMU
 
@@ -2653,6 +2654,13 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
QEMU_MIGRATION_COOKIE_NBD)))
 goto cleanup;
 
+if (STREQ(protocol, rdma)  !vm-def-mem.hard_limit) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+   _(cannot start RDMA migration with no memory hard 
+ limit set));
+goto cleanup;
+}
+
 if (qemuMigrationJobStart(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN)  0)
 goto cleanup;
 qemuMigrationJobSetPhase(driver, vm, QEMU_MIGRATION_PHASE_PREPARE);
@@ -2696,6 +2704,11 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 QEMU_ASYNC_JOB_MIGRATION_IN)  0)
 goto stop;
 
+if (STREQ(protocol, rdma) 
+virProcessSetMaxMemLock(vm-pid, vm-def-mem.hard_limit  10)  0) {
+goto stop;
+}
+
 if (mig-lockState) {
 VIR_DEBUG(Received lockstate %s, mig-lockState);
 VIR_FREE(priv-lockState);
@@ -2926,7 +2939,8 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver,
 if (!(uri = qemuMigrationParseURI(uri_in, well_formed_uri)))
 goto cleanup;
 
-if (STRNEQ(uri-scheme, tcp)) {
+if (STRNEQ(uri-scheme, tcp) 
+STRNEQ(uri-scheme, rdma)) {
 virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
_(unsupported scheme %s in migration URI %s),
uri-scheme, uri_in);
@@ -3545,6 +3559,11 @@ qemuMigrationRun(virQEMUDriverPtr driver,
 
 switch (spec-destType) {
 case MIGRATION_DEST_HOST:
+if (STREQ(spec-dest.host.protocol, rdma) 
+

[libvirt] [PATCH 4/6] qemu: Add RDMA migration capabilities

2014-09-17 Thread Jiri Denemark
Signed-off-by: Jiri Denemark jdene...@redhat.com
---

Notes:
Version 3:
- separated from qemu: RDMA migration support
- switched from version based to feature based probing
- fixed existing capabilities tests
- added new capabilities test for QEMU 2.1.1 which supports
  rdma-pin-all migration capability

 src/qemu/qemu_capabilities.c |   32 +-
 src/qemu/qemu_capabilities.h |1 +
 src/qemu/qemu_monitor.c  |   22 +-
 src/qemu/qemu_monitor.h  |3 +
 src/qemu/qemu_monitor_json.c |   44 +-
 src/qemu/qemu_monitor_json.h |2 +
 tests/qemucapabilitiesdata/caps_1.2.2-1.replies  |   10 +
 tests/qemucapabilitiesdata/caps_1.3.1-1.replies  |   10 +
 tests/qemucapabilitiesdata/caps_1.4.2-1.replies  |   10 +
 tests/qemucapabilitiesdata/caps_1.5.3-1.replies  |   10 +
 tests/qemucapabilitiesdata/caps_1.6.0-1.replies  |   22 +
 tests/qemucapabilitiesdata/caps_1.6.50-1.replies |   22 +
 tests/qemucapabilitiesdata/caps_2.1.1-1.caps |  162 ++
 tests/qemucapabilitiesdata/caps_2.1.1-1.replies  | 3264 ++
 tests/qemucapabilitiestest.c |1 +
 15 files changed, 3602 insertions(+), 13 deletions(-)
 create mode 100644 tests/qemucapabilitiesdata/caps_2.1.1-1.caps
 create mode 100644 tests/qemucapabilitiesdata/caps_2.1.1-1.replies

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 9f8868d..30b3c5d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -269,6 +269,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
 
   splash-timeout, /* 175 */
   iothread,
+  migrate-rdma,
 );
 
 
@@ -993,9 +994,9 @@ virCapsPtr virQEMUCapsInit(virQEMUCapsCachePtr cache)
 if (virQEMUCapsInitPages(caps)  0)
 VIR_WARN(Failed to get pages info);
 
-/* Add domain migration transport URI */
-virCapabilitiesAddHostMigrateTransport(caps,
-   tcp);
+/* Add domain migration transport URIs */
+virCapabilitiesAddHostMigrateTransport(caps, tcp);
+virCapabilitiesAddHostMigrateTransport(caps, rdma);
 
 /* QEMU can support pretty much every arch that exists,
  * so just probe for them all - we gracefully fail
@@ -1435,6 +1436,10 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = {
 { rtc-reset-reinjection, QEMU_CAPS_RTC_RESET_REINJECTION },
 };
 
+struct virQEMUCapsStringFlags virQEMUCapsMigration[] = {
+{ rdma-pin-all, QEMU_CAPS_MIGRATE_RDMA },
+};
+
 struct virQEMUCapsStringFlags virQEMUCapsEvents[] = {
 { BALLOON_CHANGE, QEMU_CAPS_BALLOON_EVENT },
 { SPICE_MIGRATE_COMPLETED, QEMU_CAPS_SEAMLESS_MIGRATION },
@@ -2476,6 +2481,25 @@ virQEMUCapsProbeQMPCommandLine(virQEMUCapsPtr qemuCaps,
 return 0;
 }
 
+static int
+virQEMUCapsProbeQMPMigrationCapabilities(virQEMUCapsPtr qemuCaps,
+ qemuMonitorPtr mon)
+{
+char **caps = NULL;
+int ncaps;
+
+if ((ncaps = qemuMonitorGetMigrationCapabilities(mon, caps))  0)
+return -1;
+
+virQEMUCapsProcessStringFlags(qemuCaps,
+  ARRAY_CARDINALITY(virQEMUCapsMigration),
+  virQEMUCapsMigration,
+  ncaps, caps);
+virQEMUCapsFreeStringList(ncaps, caps);
+
+return 0;
+}
+
 int virQEMUCapsProbeQMP(virQEMUCapsPtr qemuCaps,
 qemuMonitorPtr mon)
 {
@@ -3168,6 +3192,8 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
 goto cleanup;
 if (virQEMUCapsProbeQMPCommandLine(qemuCaps, mon)  0)
 goto cleanup;
+if (virQEMUCapsProbeQMPMigrationCapabilities(qemuCaps, mon)  0)
+goto cleanup;
 
 ret = 0;
  cleanup:
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 0980c00..6a8b84f 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -216,6 +216,7 @@ typedef enum {
 QEMU_CAPS_RTC_RESET_REINJECTION = 174, /* rtc-reset-reinjection monitor 
command */
 QEMU_CAPS_SPLASH_TIMEOUT = 175, /* -boot splash-time */
 QEMU_CAPS_OBJECT_IOTHREAD= 176, /* -object iothread */
+QEMU_CAPS_MIGRATE_RDMA   = 177, /* have rdma migration */
 
 QEMU_CAPS_LAST,   /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 31ab37d..0f48398 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -121,7 +121,7 @@ VIR_ENUM_IMPL(qemuMonitorMigrationStatus,
 
 VIR_ENUM_IMPL(qemuMonitorMigrationCaps,
   QEMU_MONITOR_MIGRATION_CAPS_LAST,
-  xbzrle, auto-converge)
+  xbzrle, auto-converge, rdma-pin-all)
 
 VIR_ENUM_IMPL(qemuMonitorVMStatus,
   QEMU_MONITOR_VM_STATUS_LAST,
@@ -3762,6 +3762,26 @@ char 

[libvirt] [PATCH 1/6] qemu: Expose additional migration statistics

2014-09-17 Thread Jiri Denemark
From: Michael R. Hines mrhi...@us.ibm.com

RDMA migration uses the 'setup' state in QEMU to optionally lock
all memory before the migration starts. The total time spent in
this state is exposed as VIR_DOMAIN_JOB_SETUP_TIME.

Additionally, QEMU also exports migration throughput (mbps) for both
memory and disk, so let's add them too: VIR_DOMAIN_JOB_MEMORY_BPS,
VIR_DOMAIN_JOB_DISK_BPS.

Signed-off-by: Michael R. Hines mrhi...@us.ibm.com
Signed-off-by: Jiri Denemark jdene...@redhat.com
---

Notes:
Version 3:
- changed MBPS to BPS
- added support for both memory and disk BPS
- changed BPS to ULLONG
- added code to transfer the new statistics to the destination
  daemon when migration completes
- added the new parameters to virsh

 include/libvirt/libvirt.h.in | 25 +
 src/qemu/qemu_domain.c   | 18 ++
 src/qemu/qemu_migration.c| 17 +
 src/qemu/qemu_monitor.h  |  9 +
 src/qemu/qemu_monitor_json.c | 17 +
 tools/virsh-domain.c | 27 +++
 6 files changed, 113 insertions(+)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index c2f9d26..702f797 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -4388,6 +4388,15 @@ int virDomainAbortJob(virDomainPtr dom);
 #define VIR_DOMAIN_JOB_DOWNTIME downtime
 
 /**
+ * VIR_DOMAIN_JOB_SETUP_TIME:
+ *
+ * virDomainGetJobStats field: total time in milliseconds spent preparing
+ * the migration in the 'setup' phase before the iterations begin, as
+ * VIR_TYPED_PARAM_ULLONG.
+ */
+#define VIR_DOMAIN_JOB_SETUP_TIME   setup_time
+
+/**
  * VIR_DOMAIN_JOB_DATA_TOTAL:
  *
  * virDomainGetJobStats field: total number of bytes supposed to be
@@ -4485,6 +4494,14 @@ int virDomainAbortJob(virDomainPtr dom);
 #define VIR_DOMAIN_JOB_MEMORY_NORMAL_BYTES  memory_normal_bytes
 
 /**
+ * VIR_DOMAIN_JOB_MEMORY_BPS:
+ *
+ * virDomainGetJobStats field: network throughput used while migrating
+ * memory in Bytes per second, as VIR_TYPED_PARAM_ULLONG.
+ */
+#define VIR_DOMAIN_JOB_MEMORY_BPS   memory_bps
+
+/**
  * VIR_DOMAIN_JOB_DISK_TOTAL:
  *
  * virDomainGetJobStats field: as VIR_DOMAIN_JOB_DATA_TOTAL but only
@@ -4515,6 +4532,14 @@ int virDomainAbortJob(virDomainPtr dom);
 #define VIR_DOMAIN_JOB_DISK_REMAINING   disk_remaining
 
 /**
+ * VIR_DOMAIN_JOB_DISK_BPS:
+ *
+ * virDomainGetJobStats field: network throughput used while migrating
+ * disks in Bytes per second, as VIR_TYPED_PARAM_ULLONG.
+ */
+#define VIR_DOMAIN_JOB_DISK_BPS disk_bps
+
+/**
  * VIR_DOMAIN_JOB_COMPRESSION_CACHE:
  *
  * virDomainGetJobStats field: size of the cache (in bytes) used for
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 863ab09..9b3edd7 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -304,6 +304,12 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo,
 status-downtime)  0)
 goto error;
 
+if (status-setup_time_set 
+virTypedParamsAddULLong(par, npar, maxpar,
+VIR_DOMAIN_JOB_SETUP_TIME,
+status-setup_time)  0)
+goto error;
+
 if (virTypedParamsAddULLong(par, npar, maxpar,
 VIR_DOMAIN_JOB_DATA_TOTAL,
 status-ram_total +
@@ -329,6 +335,12 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo,
 status-ram_remaining)  0)
 goto error;
 
+if (status-ram_bps 
+virTypedParamsAddULLong(par, npar, maxpar,
+VIR_DOMAIN_JOB_MEMORY_BPS,
+status-ram_bps)  0)
+goto error;
+
 if (status-ram_duplicate_set) {
 if (virTypedParamsAddULLong(par, npar, maxpar,
 VIR_DOMAIN_JOB_MEMORY_CONSTANT,
@@ -353,6 +365,12 @@ qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo,
 status-disk_remaining)  0)
 goto error;
 
+if (status-disk_bps 
+virTypedParamsAddULLong(par, npar, maxpar,
+VIR_DOMAIN_JOB_DISK_BPS,
+status-disk_bps)  0)
+goto error;
+
 if (status-xbzrle_set) {
 if (virTypedParamsAddULLong(par, npar, maxpar,
 VIR_DOMAIN_JOB_COMPRESSION_CACHE,
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index ce1a5cd..d738f9b 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -636,6 +636,10 @@ qemuMigrationCookieStatisticsXMLFormat(virBufferPtr buf,
 virBufferAsprintf(buf, %1$s%2$llu/%1$s\n,
   VIR_DOMAIN_JOB_DOWNTIME,
   status-downtime);
+if (status-setup_time_set)
+virBufferAsprintf(buf, 

Re: [libvirt] [PATCH 0/6] RDMA migration support

2014-09-17 Thread Jiri Denemark
On Wed, Sep 17, 2014 at 16:53:02 +0200, Jiri Denemark wrote:
 This is a modified version of RDMA migration patches sent back in
 January by Michael R. Hines. See individual patches for (numerous)
 changes since v2.
 
 Jiri Denemark (3):
   qemu: Fix old tcp:host URIs more cleanly
   qemu: Prepare support for arbitrary migration protocol
   qemu: Add RDMA migration capabilities
 
 Michael R. Hines (3):
   qemu: Expose additional migration statistics
   qemu: RDMA migration support
   qemu: Memory pre-pinning support for RDMA migration

And this whole series should have been obviously marked as v3...

Jirka

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


Re: [libvirt] [PATCH 4/5] conf: add options for disabling segment offloading

2014-09-17 Thread Daniel P. Berrange
On Mon, Sep 15, 2014 at 04:30:46PM -0600, Eric Blake wrote:
 On 09/11/2014 05:43 AM, Ján Tomko wrote:
  Add the following attributes:
  csum, gso, guest_tso4, guest_tso6, guest_ecn
  to the driver element of network interface
  which control the virtio-net device properties
  of the same names.
  ---
   docs/formatdomain.html.in  | 27 
   docs/schemas/domaincommon.rng  | 25 +++
   src/conf/domain_conf.c | 81 
  ++
   src/conf/domain_conf.h |  5 ++
   .../qemuxml2argv-net-virtio-disable-offloads.xml   | 32 +
   tests/qemuxml2xmltest.c|  1 +
   6 files changed, 171 insertions(+)
   create mode 100644 
  tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml
  
  diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
  index a2ea758..5b2758a 100644
  --- a/docs/formatdomain.html.in
  +++ b/docs/formatdomain.html.in
  @@ -3847,6 +3847,12 @@ qemu-kvm -net nic,model=? /dev/null
 lt;model type='virtio'/gt;
 blt;driver name='vhost' txmode='iothread' ioeventfd='on' 
  event_idx='off' queues='5'/gt;/b
   lt;/interfacegt;
  +lt;interface type='network'gt;
  +  lt;source network='default'/gt;
  +  lt;target dev='vnet2'/gt;
  +  lt;model type='virtio'/gt;
  +  blt;driver csum='off' gso='off' guest_tso4='off' guest_tso6='off' 
  guest_ecn='off'/gt;/b
  +lt;/interfacegt;
 
 Are we stuck with names with underscores in our XML?  I'm still not sure
 if we've come up with the best naming for exposing all these knobs.

I'm not really convinced having a 'guest_' prefix really buys
us anything here, since there's no naming clash to avoid. Why
don't we just kill the 'guest_' prefixes.

NB, remember that precisely matching QEMU naming is a non-goal,
we should be designing something that makes sense in general.

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

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

Re: [libvirt] [PATCH v2] rpc: make daemon spawning a bit more intelligent

2014-09-17 Thread Martin Kletzander

On Wed, Sep 17, 2014 at 10:59:21AM -0400, John Ferlan wrote:



On 09/17/2014 10:00 AM, Martin Kletzander wrote:

On Tue, Sep 16, 2014 at 10:22:53AM -0400, John Ferlan wrote:



On 09/16/2014 05:16 AM, Martin Kletzander wrote:

This way it behaves more like the daemon itself does (acquiring a
pidfile, deleting the socket before binding, etc.).

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=927369
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1138604

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 src/rpc/virnetsocket.c | 65 +++---
 1 file changed, 57 insertions(+), 8 deletions(-)



The error path/retry logic needs a tweak... I added some inline thinking
since we don't have a virtual whiteboard to share on this!



Yes, it does.  I didn't think it through from scratch, just adjusted
to your comments.  This time I went through it few times.  Just let me
confirm I understood what you meant everywhere.



You did :-)

...snip...



Wow, I just reached this part of the mail when I wrote it already :)


Funny how that happens.



My current diff to the previous version looks like this:

diff --git i/src/rpc/virnetsocket.c w/src/rpc/virnetsocket.c
index 7be1492..e0efb14 100644
--- i/src/rpc/virnetsocket.c
+++ w/src/rpc/virnetsocket.c
@@ -596,7 +596,6 @@ int virNetSocketNewConnectUNIX(const char *path,
 goto error;

 pidfd = virPidFileAcquirePath(pidpath, false, getpid());
-VIR_FREE(pidpath);
 if (pidfd  0) {
 /*
  * This can happen in a very rare case of two clients
  spawning two
@@ -650,6 +649,7 @@ int virNetSocketNewConnectUNIX(const char *path,
  * time without spawning the daemon.
  */
 spawnDaemon = false;
+virPidFileDeletePath(pidpath);
 VIR_FORCE_CLOSE(pidfd);
 VIR_FORCE_CLOSE(passfd);
 goto retry;
@@ -674,6 +674,7 @@ int virNetSocketNewConnectUNIX(const char *path,
  * virCommandHook inside a virNetSocketForkDaemon().
  */
 VIR_FORCE_CLOSE(pidfd);
+pidfd = -1;


VIR_FORCE_CLOSE() will do this for you


 if (virNetSocketForkDaemon(binary, passfd)  0)
 goto error;
 }
@@ -687,10 +688,12 @@ int virNetSocketNewConnectUNIX(const char *path,
 if (!(*retsock = virNetSocketNew(localAddr, remoteAddr, true,
 fd, -1, 0)))
 goto error;

+VIR_FREE(pidpath);
+
 return 0;

  error:
-if (pidfd  0)
+if (pidfd = 0)
 virPidFileDeletePath(pidpath);
 VIR_FREE(pidpath);
 VIR_FORCE_CLOSE(fd);
--

Is that fine or should I use that goto error; everywhere after
acquiring the pidfile or is it better for you to see it in another
version?



This is fine - I think things are now covered.

ACK



Thanks for you thorough review, I squashed it in, remove that one
unnecessary pidfd = -1 and pushed it.

Martin


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

Re: [libvirt] [PATCH v2 0/2] Expose UEFI binary path

2014-09-17 Thread Eric Blake
On 09/17/2014 06:15 AM, Michal Privoznik wrote:
 This is practically reworked v1 from Cole.
 
 Cole Robinson (1):
   domaincaps: Expose UEFI binary path, if it exists
 
 Michal Privoznik (1):
   qemu_capabilities: Change virQEMUCapsFillDomainCaps signature
 
  docs/formatdomaincaps.html.in  |  6 ++
  docs/schemas/domaincaps.rng| 17 --
  src/conf/domain_capabilities.c | 29 ++
  src/conf/domain_capabilities.h |  8 +++
  src/qemu/qemu_capabilities.c   | 53 +
  src/qemu/qemu_capabilities.h   |  9 ++-
  src/qemu/qemu_driver.c |  7 ++-
  tests/domaincapsschemadata/domaincaps-full.xml |  2 +
  .../domaincaps-qemu_1.6.50-1.xml   |  1 +
  tests/domaincapstest.c | 66 
 ++
  10 files changed, 167 insertions(+), 31 deletions(-)

I'm now seeing test failures:

FAIL: domaincapstest


TEST: domaincapstest
 1) basic ... OK
 2) full  ... OK
 3) qemu_1.6.50-1 ...
Offset 196
Expect [value/usr/share/OVMF/OVMF_CODE.fd/value
  e]
Actual [e]

... FAILED

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



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

[libvirt] [PATCH 0/5] Refactor save image opening and add restore hook

2014-09-17 Thread Peter Krempa
This series refactors (splits) the save image open function into separate chunks
and introduces a filter-hook that is called when restoring a save image.

Peter Krempa (5):
  qemu: save image: Split out user provided XML checker
  qemu: save image: Add possibility to return XML stored in the image
  qemu: save image: Split out new definition check/update
  qemu: save image: Split out checks done only when editing the save img
  qemu: hook: Provide hook when restoring a domain save image

 docs/hooks.html.in |  11 +++
 src/qemu/qemu_driver.c | 261 ++---
 src/util/virhook.c |   3 +-
 src/util/virhook.h |   1 +
 4 files changed, 195 insertions(+), 81 deletions(-)

-- 
2.1.0

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


[libvirt] [PATCH 3/5] qemu: save image: Split out new definition check/update

2014-09-17 Thread Peter Krempa
Split out the call to the update method only to places where it is
actually used rather than having a mega-method that does all the stuff.
---
 src/qemu/qemu_driver.c | 26 +++---
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a276ea5..0151fd2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5495,16 +5495,6 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver,
 VIR_DOMAIN_XML_INACTIVE)))
 goto error;

-if (xmlin) {
-virDomainDefPtr tmp;
-
-if (!(tmp = qemuDomainSaveImageUpdateDef(driver, def, xmlin)))
-goto error;
-
-virDomainDefFree(def);
-def = tmp;
-}
-
 if (xmlout)
 *xmlout = xml;
 else
@@ -5647,6 +5637,7 @@ qemuDomainRestoreFlags(virConnectPtr conn,
 {
 virQEMUDriverPtr driver = conn-privateData;
 virDomainDefPtr def = NULL;
+virDomainDefPtr newdef = NULL;
 virDomainObjPtr vm = NULL;
 int fd = -1;
 int ret = -1;
@@ -5673,6 +5664,14 @@ qemuDomainRestoreFlags(virConnectPtr conn,
 if (virDomainRestoreFlagsEnsureACL(conn, def)  0)
 goto cleanup;

+if (dxml) {
+if (!(newdef = qemuDomainSaveImageUpdateDef(driver, def, dxml)))
+goto cleanup;
+
+virDomainDefFree(def);
+def = newdef;
+}
+
 if (!(vm = virDomainObjListAdd(driver-domains, def,
driver-xmlopt,
VIR_DOMAIN_OBJ_LIST_ADD_LIVE |
@@ -5749,6 +5748,7 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const 
char *path,
 virQEMUDriverPtr driver = conn-privateData;
 int ret = -1;
 virDomainDefPtr def = NULL;
+virDomainDefPtr newdef = NULL;
 int fd = -1;
 virQEMUSaveHeader header;
 char *xml = NULL;
@@ -5776,7 +5776,10 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const 
char *path,
 if (virDomainSaveImageDefineXMLEnsureACL(conn, def)  0)
 goto cleanup;

-xml = qemuDomainDefFormatXML(driver, def,
+if (!(newdef = qemuDomainSaveImageUpdateDef(driver, def, dxml)))
+goto cleanup;
+
+xml = qemuDomainDefFormatXML(driver, newdef,
  VIR_DOMAIN_XML_INACTIVE |
  VIR_DOMAIN_XML_SECURE |
  VIR_DOMAIN_XML_MIGRATABLE);
@@ -5807,6 +5810,7 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const 
char *path,

  cleanup:
 virDomainDefFree(def);
+virDomainDefFree(newdef);
 VIR_FORCE_CLOSE(fd);
 VIR_FREE(xml);
 return ret;
-- 
2.1.0

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


[libvirt] [PATCH 2/5] qemu: save image: Add possibility to return XML stored in the image

2014-09-17 Thread Peter Krempa
Add a new parameter that will allow to return the XML stored in the save
image for further manipulation and adjust the callers. This option will
be used in later patches.
---
 src/qemu/qemu_driver.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e41a08e..a276ea5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5382,6 +5382,7 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver,
 const char *path,
 virDomainDefPtr *ret_def,
 virQEMUSaveHeaderPtr ret_header,
+char **xmlout,
 bool bypass_cache,
 virFileWrapperFdPtr *wrapperFd,
 const char *xmlin, int state, bool edit,
@@ -5504,7 +5505,10 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver,
 def = tmp;
 }

-VIR_FREE(xml);
+if (xmlout)
+*xmlout = xml;
+else
+VIR_FREE(xml);

 *ret_def = def;
 *ret_header = header;
@@ -5660,7 +5664,7 @@ qemuDomainRestoreFlags(virConnectPtr conn,
 else if (flags  VIR_DOMAIN_SAVE_PAUSED)
 state = 0;

-fd = qemuDomainSaveImageOpen(driver, path, def, header,
+fd = qemuDomainSaveImageOpen(driver, path, def, header, NULL,
  (flags  VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0,
  wrapperFd, dxml, state, false, false);
 if (fd  0)
@@ -5721,8 +5725,8 @@ qemuDomainSaveImageGetXMLDesc(virConnectPtr conn, const 
char *path,
 /* We only take subset of virDomainDefFormat flags.  */
 virCheckFlags(VIR_DOMAIN_XML_SECURE, NULL);

-fd = qemuDomainSaveImageOpen(driver, path, def, header, false, NULL,
- NULL, -1, false, false);
+fd = qemuDomainSaveImageOpen(driver, path, def, header, NULL,
+ false, NULL, NULL, -1, false, false);

 if (fd  0)
 goto cleanup;
@@ -5759,8 +5763,8 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const 
char *path,
 else if (flags  VIR_DOMAIN_SAVE_PAUSED)
 state = 0;

-fd = qemuDomainSaveImageOpen(driver, path, def, header, false, NULL,
- dxml, state, true, false);
+fd = qemuDomainSaveImageOpen(driver, path, def, header, NULL,
+ false, NULL, dxml, state, true, false);

 if (fd  0) {
 /* Check for special case of no change needed.  */
@@ -5824,7 +5828,7 @@ qemuDomainObjRestore(virConnectPtr conn,
 virQEMUSaveHeader header;
 virFileWrapperFdPtr wrapperFd = NULL;

-fd = qemuDomainSaveImageOpen(driver, path, def, header,
+fd = qemuDomainSaveImageOpen(driver, path, def, header, NULL,
  bypass_cache, wrapperFd, NULL, -1, false,
  true);
 if (fd  0) {
-- 
2.1.0

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


[libvirt] [PATCH 1/5] qemu: save image: Split out user provided XML checker

2014-09-17 Thread Peter Krempa
Extract code used to check save image XMLs provided by users to separate
use.
---
 src/qemu/qemu_driver.c | 100 -
 1 file changed, 66 insertions(+), 34 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 15ad64d..e41a08e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5311,6 +5311,68 @@ static int qemuNodeGetSecurityModel(virConnectPtr conn,
 return ret;
 }

+
+/**
+ * qemuDomainSaveImageUpdateDef:
+ * @driver: qemu driver data
+ * @def: def of the domain from the save image
+ * @newxml: user provided replacement XML
+ *
+ * Returns the new domain definition in case @newxml is ABI compatible with the
+ * guest.
+ */
+static virDomainDefPtr
+qemuDomainSaveImageUpdateDef(virQEMUDriverPtr driver,
+ virDomainDefPtr def,
+ const char *newxml)
+{
+virDomainDefPtr ret = NULL;
+virDomainDefPtr newdef_migr = NULL;
+virDomainDefPtr newdef = NULL;
+virCapsPtr caps = NULL;
+
+if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+goto cleanup;
+
+if (!(newdef = virDomainDefParseString(newxml, caps, driver-xmlopt,
+   QEMU_EXPECTED_VIRT_TYPES,
+   VIR_DOMAIN_XML_INACTIVE)))
+goto cleanup;
+
+if (!(newdef_migr = qemuDomainDefCopy(driver,
+  newdef,
+  VIR_DOMAIN_XML_MIGRATABLE)))
+goto cleanup;
+
+if (!virDomainDefCheckABIStability(def, newdef_migr)) {
+virResetLastError();
+
+/* Due to a bug in older version of external snapshot creation
+ * code, the XML saved in the save image was not a migratable
+ * XML. To ensure backwards compatibility with the change of the
+ * saved XML type, we need to check the ABI compatibility against
+ * the user provided XML if the check against the migratable XML
+ * fails. Snapshots created prior to v1.1.3 have this issue. */
+if (!virDomainDefCheckABIStability(def, newdef))
+goto cleanup;
+
+/* use the user provided XML */
+ret = newdef;
+newdef = NULL;
+} else {
+ret = newdef_migr;
+newdef_migr = NULL;
+}
+
+ cleanup:
+virObjectUnref(caps);
+virDomainDefFree(newdef);
+virDomainDefFree(newdef_migr);
+
+return ret;
+}
+
+
 /* Return -1 on most failures after raising error, -2 if edit was specified
  * but xmlin and state (-1 for no change, 0 for paused, 1 for running) do
  * not represent any changes (no error raised), -3 if corrupt image was
@@ -5431,45 +5493,15 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver,
 QEMU_EXPECTED_VIRT_TYPES,
 VIR_DOMAIN_XML_INACTIVE)))
 goto error;
-if (xmlin) {
-virDomainDefPtr def2 = NULL;
-virDomainDefPtr newdef = NULL;

-if (!(def2 = virDomainDefParseString(xmlin, caps, driver-xmlopt,
- QEMU_EXPECTED_VIRT_TYPES,
- VIR_DOMAIN_XML_INACTIVE)))
-goto error;
+if (xmlin) {
+virDomainDefPtr tmp;

-newdef = qemuDomainDefCopy(driver, def2, VIR_DOMAIN_XML_MIGRATABLE);
-if (!newdef) {
-virDomainDefFree(def2);
+if (!(tmp = qemuDomainSaveImageUpdateDef(driver, def, xmlin)))
 goto error;
-}
-
-if (!virDomainDefCheckABIStability(def, newdef)) {
-virDomainDefFree(newdef);
-virResetLastError();
-
-/* Due to a bug in older version of external snapshot creation
- * code, the XML saved in the save image was not a migratable
- * XML. To ensure backwards compatibility with the change of the
- * saved XML type, we need to check the ABI compatibility against
- * the user provided XML if the check against the migratable XML
- * fails. Snapshots created prior to v1.1.3 have this issue. */
-if (!virDomainDefCheckABIStability(def, def2)) {
-virDomainDefFree(def2);
-goto error;
-}
-
-/* use the user provided XML */
-newdef = def2;
-def2 = NULL;
-} else {
-virDomainDefFree(def2);
-}

 virDomainDefFree(def);
-def = newdef;
+def = tmp;
 }

 VIR_FREE(xml);
-- 
2.1.0

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


[libvirt] [PATCH 4/5] qemu: save image: Split out checks done only when editing the save img

2014-09-17 Thread Peter Krempa
Move them to the single corresponding function rather than having them
in the common chunk of code.
---
 src/qemu/qemu_driver.c | 76 +++---
 1 file changed, 41 insertions(+), 35 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0151fd2..1d82e93 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5373,10 +5373,22 @@ qemuDomainSaveImageUpdateDef(virQEMUDriverPtr driver,
 }


-/* Return -1 on most failures after raising error, -2 if edit was specified
- * but xmlin and state (-1 for no change, 0 for paused, 1 for running) do
- * not represent any changes (no error raised), -3 if corrupt image was
- * unlinked (no error raised), and opened fd on success.  */
+/**
+ * qemuDomainSaveImageOpen:
+ * @driver: qemu driver data
+ * @path: path of the save image
+ * @ret_def: returns domain definition created from the XML stored in the image
+ * @ret_header: returns structure filled with data from the image header
+ * @xmlout: returns the XML from the image file (may be NULL)
+ * @bypass_cache: bypass cache when opening the file
+ * @wrapperFd: returns the file wrapper structure
+ * @open_write: open the file for writing (for updates)
+ * @unlink_corrupt: remove the image file if it is corrupted
+ *
+ * Returns the opened fd of the save image file and fills the apropriate fields
+ * on success. On error returns -1 on most failures, -3 if corrupt image was
+ * unlinked (no error raised).
+ */
 static int ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4)
 qemuDomainSaveImageOpen(virQEMUDriverPtr driver,
 const char *path,
@@ -5385,14 +5397,14 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver,
 char **xmlout,
 bool bypass_cache,
 virFileWrapperFdPtr *wrapperFd,
-const char *xmlin, int state, bool edit,
+bool open_write,
 bool unlink_corrupt)
 {
 int fd = -1;
 virQEMUSaveHeader header;
 char *xml = NULL;
 virDomainDefPtr def = NULL;
-int oflags = edit ? O_RDWR : O_RDONLY;
+int oflags = open_write ? O_RDWR : O_RDONLY;
 virCapsPtr caps = NULL;

 if (bypass_cache) {
@@ -5477,18 +5489,6 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver,
 goto error;
 }

-if (edit  STREQ(xml, xmlin) 
-(state  0 || state == header.was_running)) {
-VIR_FREE(xml);
-if (VIR_CLOSE(fd)  0) {
-virReportSystemError(errno, _(cannot close file: %s), path);
-goto error;
-}
-return -2;
-}
-if (state = 0)
-header.was_running = state;
-
 /* Create a domain from this XML */
 if (!(def = virDomainDefParseString(xml, caps, driver-xmlopt,
 QEMU_EXPECTED_VIRT_TYPES,
@@ -5643,21 +5643,15 @@ qemuDomainRestoreFlags(virConnectPtr conn,
 int ret = -1;
 virQEMUSaveHeader header;
 virFileWrapperFdPtr wrapperFd = NULL;
-int state = -1;

 virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE |
   VIR_DOMAIN_SAVE_RUNNING |
   VIR_DOMAIN_SAVE_PAUSED, -1);


-if (flags  VIR_DOMAIN_SAVE_RUNNING)
-state = 1;
-else if (flags  VIR_DOMAIN_SAVE_PAUSED)
-state = 0;
-
 fd = qemuDomainSaveImageOpen(driver, path, def, header, NULL,
  (flags  VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0,
- wrapperFd, dxml, state, false, false);
+ wrapperFd, false, false);
 if (fd  0)
 goto cleanup;

@@ -5680,6 +5674,11 @@ qemuDomainRestoreFlags(virConnectPtr conn,
 goto cleanup;
 def = NULL;

+if (flags  VIR_DOMAIN_SAVE_RUNNING)
+header.was_running = 1;
+else if (flags  VIR_DOMAIN_SAVE_PAUSED)
+header.was_running = 0;
+
 if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY)  0)
 goto cleanup;

@@ -5725,7 +5724,7 @@ qemuDomainSaveImageGetXMLDesc(virConnectPtr conn, const 
char *path,
 virCheckFlags(VIR_DOMAIN_XML_SECURE, NULL);

 fd = qemuDomainSaveImageOpen(driver, path, def, header, NULL,
- false, NULL, NULL, -1, false, false);
+ false, NULL, false, false);

 if (fd  0)
 goto cleanup;
@@ -5763,22 +5762,30 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const 
char *path,
 else if (flags  VIR_DOMAIN_SAVE_PAUSED)
 state = 0;

-fd = qemuDomainSaveImageOpen(driver, path, def, header, NULL,
- false, NULL, dxml, state, true, false);
+fd = qemuDomainSaveImageOpen(driver, path, def, header, xml,
+ false, NULL, true, false);

-if (fd  0) {
-/* Check for special case of no change needed.  */
-if (fd == -2)
-ret = 0;
+if (fd  0)
 goto cleanup;
-}

 if 

[libvirt] [PATCH 5/5] qemu: hook: Provide hook when restoring a domain save image

2014-09-17 Thread Peter Krempa
---
 docs/hooks.html.in | 11 
 src/qemu/qemu_driver.c | 69 +-
 src/util/virhook.c |  3 ++-
 src/util/virhook.h |  1 +
 4 files changed, 76 insertions(+), 8 deletions(-)

diff --git a/docs/hooks.html.in b/docs/hooks.html.in
index 07b9d49..265dbb7 100644
--- a/docs/hooks.html.in
+++ b/docs/hooks.html.in
@@ -177,6 +177,17 @@
 script returns failure or the output XML is not valid, incoming
 migration will be canceled. This hook may be used, e.g., to change
 location of disk images for incoming domains./li
+  lispan class=sinceSince 1.2.9/span, the qemu hook script is
+also called when restoring a saved image either via the API or
+automatically when restoring a managed save machine. It is called
+as: pre/etc/libvirt/hooks/qemu guest_name restore begin -/pre
+with domain XML sent to standard input of the script. In this case,
+the script acts as a filter and is supposed to modify the domain
+XML and print it out on its standard output. Empty output is
+identical to copying the input XML without changing it. In case the
+script returns failure or the output XML is not valid, restore of the
+image will be aborted. This hook may be used, e.g., to change
+location of disk images for incoming domains./li
   lispan class=sinceSince 0.9.13/span, the qemu hook script
 is also called when the libvirtd daemon restarts and reconnects
 to previously running QEMU processes. If the script fails, the
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1d82e93..2dd2e48 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5636,20 +5636,23 @@ qemuDomainRestoreFlags(virConnectPtr conn,
unsigned int flags)
 {
 virQEMUDriverPtr driver = conn-privateData;
+qemuDomainObjPrivatePtr priv = NULL;
 virDomainDefPtr def = NULL;
-virDomainDefPtr newdef = NULL;
 virDomainObjPtr vm = NULL;
+char *xml = NULL;
+char *xmlout = NULL;
 int fd = -1;
 int ret = -1;
 virQEMUSaveHeader header;
 virFileWrapperFdPtr wrapperFd = NULL;
+bool hook_taint = false;

 virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE |
   VIR_DOMAIN_SAVE_RUNNING |
   VIR_DOMAIN_SAVE_PAUSED, -1);


-fd = qemuDomainSaveImageOpen(driver, path, def, header, NULL,
+fd = qemuDomainSaveImageOpen(driver, path, def, header, xml,
  (flags  VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0,
  wrapperFd, false, false);
 if (fd  0)
@@ -5658,12 +5661,29 @@ qemuDomainRestoreFlags(virConnectPtr conn,
 if (virDomainRestoreFlagsEnsureACL(conn, def)  0)
 goto cleanup;

-if (dxml) {
-if (!(newdef = qemuDomainSaveImageUpdateDef(driver, def, dxml)))
+if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
+int hookret;
+
+if ((hookret = virHookCall(VIR_HOOK_DRIVER_QEMU, def-name,
+   VIR_HOOK_QEMU_OP_RESTORE,
+   VIR_HOOK_SUBOP_BEGIN,
+   NULL,
+   dxml ? dxml : xml,
+   xmlout))  0)
 goto cleanup;

-virDomainDefFree(def);
-def = newdef;
+if (hookret == 0  xmlout) {
+virDomainDefPtr tmp;
+
+VIR_DEBUG(Using hook-filtered domain XML: %s, xmlout);
+
+if (!(tmp = qemuDomainSaveImageUpdateDef(driver, def, xmlout)))
+goto cleanup;
+
+virDomainDefFree(def);
+def = tmp;
+hook_taint = true;
+}
 }

 if (!(vm = virDomainObjListAdd(driver-domains, def,
@@ -5679,6 +5699,11 @@ qemuDomainRestoreFlags(virConnectPtr conn,
 else if (flags  VIR_DOMAIN_SAVE_PAUSED)
 header.was_running = 0;

+if (hook_taint) {
+priv = vm-privateData;
+priv-hookRun = true;
+}
+
 if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY)  0)
 goto cleanup;

@@ -5697,6 +5722,8 @@ qemuDomainRestoreFlags(virConnectPtr conn,
  cleanup:
 virDomainDefFree(def);
 VIR_FORCE_CLOSE(fd);
+VIR_FREE(xml);
+VIR_FREE(xmlout);
 virFileWrapperFdFree(wrapperFd);
 if (vm)
 virObjectUnlock(vm);
@@ -5834,12 +5861,15 @@ qemuDomainObjRestore(virConnectPtr conn,
  bool bypass_cache)
 {
 virDomainDefPtr def = NULL;
+qemuDomainObjPrivatePtr priv = vm-privateData;
 int fd = -1;
 int ret = -1;
+char *xml = NULL;
+char *xmlout = NULL;
 virQEMUSaveHeader header;
 virFileWrapperFdPtr wrapperFd = NULL;

-fd = qemuDomainSaveImageOpen(driver, path, def, header, NULL,
+fd = qemuDomainSaveImageOpen(driver, path, def, header, xml,
  bypass_cache, wrapperFd, false, true);
 if (fd  0) {
 if 

Re: [libvirt] [PATCH 4/5] conf: add options for disabling segment offloading

2014-09-17 Thread Eric Blake
On 09/17/2014 08:57 AM, Daniel P. Berrange wrote:

 +  blt;driver csum='off' gso='off' guest_tso4='off' guest_tso6='off' 
 guest_ecn='off'/gt;/b
 +lt;/interfacegt;

 Are we stuck with names with underscores in our XML?  I'm still not sure
 if we've come up with the best naming for exposing all these knobs.
 
 I'm not really convinced having a 'guest_' prefix really buys
 us anything here, since there's no naming clash to avoid. Why
 don't we just kill the 'guest_' prefixes.
 
 NB, remember that precisely matching QEMU naming is a non-goal,
 we should be designing something that makes sense in general.

I agree; I'd be fine with:

 driver csum='off' gso='off' tso4='off' tso6='off' ecn='off'/

with no need for a guest sub-structure.  Yeah, we'll have to do some
glue logic to translate to qemu names, but that's what libvirt is for.

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



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

[libvirt] [PATCH] domaincapstest: Run cleanly on systems missing OVMF firmware

2014-09-17 Thread Michal Privoznik
As of f05b6a918e28 the test produces the list of paths that can
be passed to loader/ and libvirt knows about them. However,
during the process of generating the list the paths are checked
for their presence. This may produce different results on
different systems.  Therefore, the path - if missing - is
added to pretend it's there.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 tests/domaincapstest.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
index 8543963..067ad4d 100644
--- a/tests/domaincapstest.c
+++ b/tests/domaincapstest.c
@@ -118,6 +118,17 @@ fillQemuCaps(virDomainCapsPtr domCaps,
  VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT,
  VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM,
  VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO);
+
+/* Moreover, as of f05b6a918e28 we are expecting to see
+ * OVMF_CODE.fd file which may not exists everywhere. */
+if (!domCaps-os.loader.values.nvalues) {
+virDomainCapsLoaderPtr loader = domCaps-os.loader;
+
+if (fillStringValues(loader-values,
+ /usr/share/OVMF/OVMF_CODE.fd,
+ NULL)  0)
+return -1;
+}
 return 0;
 }
 #endif /* WITH_QEMU */
-- 
1.8.5.5

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


Re: [libvirt] [PATCH 4/5] conf: add options for disabling segment offloading

2014-09-17 Thread Ján Tomko
On 09/17/2014 04:57 PM, Daniel P. Berrange wrote:
 On Mon, Sep 15, 2014 at 04:30:46PM -0600, Eric Blake wrote:
 On 09/11/2014 05:43 AM, Ján Tomko wrote:
 Add the following attributes:
 csum, gso, guest_tso4, guest_tso6, guest_ecn
 to the driver element of network interface
 which control the virtio-net device properties
 of the same names.
 ---
  docs/formatdomain.html.in  | 27 
  docs/schemas/domaincommon.rng  | 25 +++
  src/conf/domain_conf.c | 81 
 ++
  src/conf/domain_conf.h |  5 ++
  .../qemuxml2argv-net-virtio-disable-offloads.xml   | 32 +
  tests/qemuxml2xmltest.c|  1 +
  6 files changed, 171 insertions(+)
  create mode 100644 
 tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml

 diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
 index a2ea758..5b2758a 100644
 --- a/docs/formatdomain.html.in
 +++ b/docs/formatdomain.html.in
 @@ -3847,6 +3847,12 @@ qemu-kvm -net nic,model=? /dev/null
lt;model type='virtio'/gt;
blt;driver name='vhost' txmode='iothread' ioeventfd='on' 
 event_idx='off' queues='5'/gt;/b
  lt;/interfacegt;
 +lt;interface type='network'gt;
 +  lt;source network='default'/gt;
 +  lt;target dev='vnet2'/gt;
 +  lt;model type='virtio'/gt;
 +  blt;driver csum='off' gso='off' guest_tso4='off' guest_tso6='off' 
 guest_ecn='off'/gt;/b
 +lt;/interfacegt;

 Are we stuck with names with underscores in our XML?  I'm still not sure
 if we've come up with the best naming for exposing all these knobs.
 
 I'm not really convinced having a 'guest_' prefix really buys
 us anything here, since there's no naming clash to avoid. Why
 don't we just kill the 'guest_' prefixes.

The clash is in the options I didn't expose:
http://git.qemu.org/?p=qemu.git;a=blob;f=include/hw/virtio/virtio-net.h;h=6ceb5aa92

because they weren't requested by the (private :() bug 1139364

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] domaincapstest: Run cleanly on systems missing OVMF firmware

2014-09-17 Thread Martin Kletzander

On Wed, Sep 17, 2014 at 05:32:03PM +0200, Michal Privoznik wrote:

As of f05b6a918e28 the test produces the list of paths that can
be passed to loader/ and libvirt knows about them. However,
during the process of generating the list the paths are checked
for their presence. This may produce different results on
different systems.  Therefore, the path - if missing - is
added to pretend it's there.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tests/domaincapstest.c | 11 +++
1 file changed, 11 insertions(+)

diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
index 8543963..067ad4d 100644
--- a/tests/domaincapstest.c
+++ b/tests/domaincapstest.c
@@ -118,6 +118,17 @@ fillQemuCaps(virDomainCapsPtr domCaps,
 VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT,
 VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM,
 VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO);
+
+/* Moreover, as of f05b6a918e28 we are expecting to see
+ * OVMF_CODE.fd file which may not exists everywhere. */
+if (!domCaps-os.loader.values.nvalues) {
+virDomainCapsLoaderPtr loader = domCaps-os.loader;
+
+if (fillStringValues(loader-values,
+ /usr/share/OVMF/OVMF_CODE.fd,
+ NULL)  0)
+return -1;
+}
return 0;
}
#endif /* WITH_QEMU */
--
1.8.5.5



ACK, build-breaker (at least for me).

Martin


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

Re: [libvirt] [PATCH] domaincapstest: Run cleanly on systems missing OVMF firmware

2014-09-17 Thread Michal Privoznik

On 17.09.2014 17:40, Martin Kletzander wrote:

On Wed, Sep 17, 2014 at 05:32:03PM +0200, Michal Privoznik wrote:

As of f05b6a918e28 the test produces the list of paths that can
be passed to loader/ and libvirt knows about them. However,
during the process of generating the list the paths are checked
for their presence. This may produce different results on
different systems.  Therefore, the path - if missing - is
added to pretend it's there.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tests/domaincapstest.c | 11 +++
1 file changed, 11 insertions(+)

diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
index 8543963..067ad4d 100644
--- a/tests/domaincapstest.c
+++ b/tests/domaincapstest.c
@@ -118,6 +118,17 @@ fillQemuCaps(virDomainCapsPtr domCaps,
 VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT,
 VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM,
 VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO);
+
+/* Moreover, as of f05b6a918e28 we are expecting to see
+ * OVMF_CODE.fd file which may not exists everywhere. */
+if (!domCaps-os.loader.values.nvalues) {
+virDomainCapsLoaderPtr loader = domCaps-os.loader;
+
+if (fillStringValues(loader-values,
+ /usr/share/OVMF/OVMF_CODE.fd,
+ NULL)  0)
+return -1;
+}
return 0;
}
#endif /* WITH_QEMU */
--
1.8.5.5



ACK, build-breaker (at least for me).



Thank you, pushed now.

Michal

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


Re: [libvirt] [PATCH v2 0/2] Use huge pages on UMA guests widely

2014-09-17 Thread Martin Kletzander

On Mon, Sep 15, 2014 at 01:48:05PM +0200, Michal Privoznik wrote:

*** BLURB HERE ***

Michal Privoznik (2):
 conf: Disallow nonexistent NUMA nodes for hugepages
 qemu: Honor hugepages for UMA domains

src/qemu/qemu_command.c| 49 --
.../qemuxml2argv-hugepages-pages4.xml  | 45 
.../qemuxml2argv-hugepages-pages5.args |  7 
.../qemuxml2argv-hugepages-pages5.xml  | 32 ++
tests/qemuxml2argvtest.c   |  3 ++
5 files changed, 132 insertions(+), 4 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages4.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages5.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages5.xml



ACK series,

Martin


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

Re: [libvirt] [PATCH 4/5] conf: add options for disabling segment offloading

2014-09-17 Thread Daniel P. Berrange
On Wed, Sep 17, 2014 at 05:36:18PM +0200, Ján Tomko wrote:
 On 09/17/2014 04:57 PM, Daniel P. Berrange wrote:
  On Mon, Sep 15, 2014 at 04:30:46PM -0600, Eric Blake wrote:
  On 09/11/2014 05:43 AM, Ján Tomko wrote:
  Add the following attributes:
  csum, gso, guest_tso4, guest_tso6, guest_ecn
  to the driver element of network interface
  which control the virtio-net device properties
  of the same names.
  ---
   docs/formatdomain.html.in  | 27 
   docs/schemas/domaincommon.rng  | 25 +++
   src/conf/domain_conf.c | 81 
  ++
   src/conf/domain_conf.h |  5 ++
   .../qemuxml2argv-net-virtio-disable-offloads.xml   | 32 +
   tests/qemuxml2xmltest.c|  1 +
   6 files changed, 171 insertions(+)
   create mode 100644 
  tests/qemuxml2argvdata/qemuxml2argv-net-virtio-disable-offloads.xml
 
  diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
  index a2ea758..5b2758a 100644
  --- a/docs/formatdomain.html.in
  +++ b/docs/formatdomain.html.in
  @@ -3847,6 +3847,12 @@ qemu-kvm -net nic,model=? /dev/null
 lt;model type='virtio'/gt;
 blt;driver name='vhost' txmode='iothread' ioeventfd='on' 
  event_idx='off' queues='5'/gt;/b
   lt;/interfacegt;
  +lt;interface type='network'gt;
  +  lt;source network='default'/gt;
  +  lt;target dev='vnet2'/gt;
  +  lt;model type='virtio'/gt;
  +  blt;driver csum='off' gso='off' guest_tso4='off' 
  guest_tso6='off' guest_ecn='off'/gt;/b
  +lt;/interfacegt;
 
  Are we stuck with names with underscores in our XML?  I'm still not sure
  if we've come up with the best naming for exposing all these knobs.
  
  I'm not really convinced having a 'guest_' prefix really buys
  us anything here, since there's no naming clash to avoid. Why
  don't we just kill the 'guest_' prefixes.
 
 The clash is in the options I didn't expose:
 http://git.qemu.org/?p=qemu.git;a=blob;f=include/hw/virtio/virtio-net.h;h=6ceb5aa92
 
 because they weren't requested by the (private :() bug 1139364

Ah, so this is why you shouldn't take the precise solution requested in
a bug too literally, and instead look at the general picture :-)

So QEMU exposes alot of stuff:

$ qemu-kvm -device virtio-net,?
virtio-net-pci.ioeventfd=on/off
virtio-net-pci.vectors=uint32
virtio-net-pci.indirect_desc=on/off
virtio-net-pci.event_idx=on/off
virtio-net-pci.any_layout=on/off
virtio-net-pci.csum=on/off
virtio-net-pci.guest_csum=on/off
virtio-net-pci.gso=on/off
virtio-net-pci.guest_tso4=on/off
virtio-net-pci.guest_tso6=on/off
virtio-net-pci.guest_ecn=on/off
virtio-net-pci.guest_ufo=on/off
virtio-net-pci.host_tso4=on/off
virtio-net-pci.host_tso6=on/off
virtio-net-pci.host_ecn=on/off
virtio-net-pci.host_ufo=on/off
virtio-net-pci.mrg_rxbuf=on/off
virtio-net-pci.status=on/off
virtio-net-pci.ctrl_vq=on/off
virtio-net-pci.ctrl_rx=on/off
virtio-net-pci.ctrl_vlan=on/off
virtio-net-pci.ctrl_rx_extra=on/off
virtio-net-pci.ctrl_mac_addr=on/off
virtio-net-pci.ctrl_guest_offloads=on/off
virtio-net-pci.mq=on/off
virtio-net-pci.mac=macaddr
virtio-net-pci.vlan=vlan
virtio-net-pci.netdev=netdev
virtio-net-pci.bootindex=int32
virtio-net-pci.x-txtimer=uint32
virtio-net-pci.x-txburst=int32
virtio-net-pci.tx=str
virtio-net-pci.addr=pci-devfn
virtio-net-pci.romfile=str
virtio-net-pci.rombar=uint32
virtio-net-pci.multifunction=on/off
virtio-net-pci.command_serr_enable=on/off


Which to me indicates that Eric's suggestion for sub-elements is a
good idea. eg go for:

  driver
guest /
host /
  /driver

and support the host bits too

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

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

Re: [libvirt] [PATCH 4/5] conf: add options for disabling segment offloading

2014-09-17 Thread Eric Blake
On 09/17/2014 09:52 AM, Daniel P. Berrange wrote:

 
 Ah, so this is why you shouldn't take the precise solution requested in
 a bug too literally, and instead look at the general picture :-)
 
 So QEMU exposes alot of stuff:
 
 $ qemu-kvm -device virtio-net,?

 virtio-net-pci.guest_tso4=on/off
 virtio-net-pci.guest_tso6=on/off
 virtio-net-pci.guest_ecn=on/off
 virtio-net-pci.guest_ufo=on/off
 virtio-net-pci.host_tso4=on/off
 virtio-net-pci.host_tso6=on/off
 virtio-net-pci.host_ecn=on/off
 virtio-net-pci.host_ufo=on/off

 
 Which to me indicates that Eric's suggestion for sub-elements is a

Wasn't my idea, but Jan's.

 good idea. eg go for:
 
   driver
 guest /
 host /
   /driver
 
 and support the host bits too

but yes, I could live with that, now that I'm seeing how many more
options there are to be exposed.

-- 
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] [PATCHv6 08/11] qemu: bulk stats: add block allocation information

2014-09-17 Thread Peter Krempa
On 09/17/14 00:37, Eric Blake wrote:
 On 09/15/2014 09:42 AM, Peter Krempa wrote:
 From: Francesco Romani from...@redhat.com

 Management software wants to be able to allocate disk space on demand.
 To support this they need keep track of the space occupation of the
 block device.  This information is reported by qemu as part of block
 stats.

 This patch extend the block information in the bulk stats with the
 allocation information.

 To keep the same behaviour a helper is extracted from
 qemuMonitorJSONGetBlockExtent in order to get per-device allocation
 information.

 Signed-off-by: Francesco Romani from...@redhat.com
 ---
  src/libvirt.c|  2 +
  src/qemu/qemu_driver.c   | 18 +
  src/qemu/qemu_monitor.h  |  1 +
  src/qemu/qemu_monitor_json.c | 91 
 ++--
  4 files changed, 92 insertions(+), 20 deletions(-)

 diff --git a/src/libvirt.c b/src/libvirt.c
 index ab10a3a..a8892a2 100644
 --- a/src/libvirt.c
 +++ b/src/libvirt.c
 @@ -21654,6 +21654,8 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
   *  unsigned long long.
   * block.num.errors - Xen only: the 'oo_req' value as
   *unsigned long long.
 + * block.num.allocation - offset of the highest written sector
 + *as unsigned long long.
 
 If we are repeating virDomainGetBlockInfo() here, I'd rather see us
 report all 3 pieces of information (allocation, capacity, physical)
 instead of just one.
 
 +typedef enum {
 +QEMU_MONITOR_BLOCK_EXTENT_ERROR_OK,
 +QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOPARENT,
 +QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOSTATS,
 +QEMU_MONITOR_BLOCK_EXTENT_ERROR_NOOFFSET
 +} qemuMonitorBlockExtentError;
 
 We require C99, so please have a trailing comma in the enum (it makes
 adding new elements easier, because they don't have to modify an
 existing line).
 

Thanks for the reviews, I've pushed this series with the changes you've
suggested except this patch as either I or Francesco will add the
requested fields.

Peter



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

[libvirt] [PATCH] CVE-2014-3633: qemu: blkiotune: Use correct definition when looking up disk

2014-09-17 Thread Peter Krempa
Live definition was used to look up the disk index while persistent one
was indexed leading to a crash in qemuDomainGetBlockIoTune. Use the
correct def and report a nice error.

Unfortunately it's accessible via read-only connection.

Introduced in: eca96694a7f992be633d48d5ca03cedc9bbc3c9aa (v0.9.8)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140724
Reported-by: Luyao Huang lhu...@redhat.com
Signed-off-by: Peter Krempa pkre...@redhat.com
---
 src/qemu/qemu_driver.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a5a49ac..209c40e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16317,9 +16317,13 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
 }

 if (flags  VIR_DOMAIN_AFFECT_CONFIG) {
-int idx = virDomainDiskIndexByName(vm-def, disk, true);
-if (idx  0)
+int idx = virDomainDiskIndexByName(persistentDef, disk, true);
+if (idx  0) {
+virReportError(VIR_ERR_INVALID_ARG,
+   _(disk '%s' was not found in the domain config),
+   disk);
 goto endjob;
+}
 reply = persistentDef-disks[idx]-blkdeviotune;
 }

-- 
2.1.0

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


Re: [libvirt] [PATCH] CVE-2014-3633: qemu: blkiotune: Use correct definition when looking up disk

2014-09-17 Thread Eric Blake
On 09/17/2014 10:25 AM, Peter Krempa wrote:
 Live definition was used to look up the disk index while persistent one
 was indexed leading to a crash in qemuDomainGetBlockIoTune. Use the
 correct def and report a nice error.
 
 Unfortunately it's accessible via read-only connection.
 

Mitigation - a read-only connection can only crash libvirtd in the cases
where the guest is hot-plugging disks without reflecting those changes
to the persistent definition.  So avoiding hotplug, or doing hotplug
where persistent is always modified alongside live definition, will
avoid the out-of-bounds access.

 Introduced in: eca96694a7f992be633d48d5ca03cedc9bbc3c9aa (v0.9.8)
 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140724
 Reported-by: Luyao Huang lhu...@redhat.com
 Signed-off-by: Peter Krempa pkre...@redhat.com
 ---
  src/qemu/qemu_driver.c | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

ACK. I can write up the libvirt security notice; we'll eventually need
this backported to all the affected maint branches.  I'll coordinate the
backport effort with you on IRC.

 
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index a5a49ac..209c40e 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -16317,9 +16317,13 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
  }
 
  if (flags  VIR_DOMAIN_AFFECT_CONFIG) {
 -int idx = virDomainDiskIndexByName(vm-def, disk, true);
 -if (idx  0)
 +int idx = virDomainDiskIndexByName(persistentDef, disk, true);
 +if (idx  0) {
 +virReportError(VIR_ERR_INVALID_ARG,
 +   _(disk '%s' was not found in the domain config),
 +   disk);
  goto endjob;
 +}
  reply = persistentDef-disks[idx]-blkdeviotune;
  }
 

-- 
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] CVE-2014-3633: qemu: blkiotune: Use correct definition when looking up disk

2014-09-17 Thread Peter Krempa
On 09/17/14 18:31, Eric Blake wrote:
 On 09/17/2014 10:25 AM, Peter Krempa wrote:
 Live definition was used to look up the disk index while persistent one
 was indexed leading to a crash in qemuDomainGetBlockIoTune. Use the
 correct def and report a nice error.

 Unfortunately it's accessible via read-only connection.

 
 Mitigation - a read-only connection can only crash libvirtd in the cases
 where the guest is hot-plugging disks without reflecting those changes
 to the persistent definition.  So avoiding hotplug, or doing hotplug
 where persistent is always modified alongside live definition, will
 avoid the out-of-bounds access.

I've added this paragraph to the commit message

 
 Introduced in: eca96694a7f992be633d48d5ca03cedc9bbc3c9aa (v0.9.8)
 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140724
 Reported-by: Luyao Huang lhu...@redhat.com
 Signed-off-by: Peter Krempa pkre...@redhat.com
 ---
  src/qemu/qemu_driver.c | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)
 
 ACK. I can write up the libvirt security notice; we'll eventually need
 this backported to all the affected maint branches.  I'll coordinate the
 backport effort with you on IRC.
 

and pushed this patch. I can handle the backport tomorrow if you don't
beat me to it.

Peter




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

Re: [libvirt] [PATCH 2/3] maint: clean up _virDomainBlockStats

2014-09-17 Thread Eric Blake
On 09/16/2014 07:19 AM, James wrote:
 I clean up all _virDomainBlockStats.
 
 Signed-off-by: James james.wangyu...@huawei.com
 Signed-off-by: Wang Rui moon.wang...@huawei.com
 ---
  src/driver.h | 2 +-
  src/libvirt.c| 2 +-
  src/lxc/lxc_driver.c | 2 +-
  src/qemu/qemu_driver.c   | 2 +-
  src/test/test_driver.c   | 2 +-
  src/xen/block_stats.c| 4 ++--
  src/xen/block_stats.h| 2 +-
  src/xen/xen_driver.c | 2 +-
  src/xen/xen_hypervisor.c | 2 +-
  src/xen/xen_hypervisor.h | 2 +-
  tools/virsh-domain-monitor.c | 2 +-
  11 files changed, 12 insertions(+), 12 deletions(-)

ACK; I'll touch up the attribution in the same manner as 1/3 and push
shortly.

-- 
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] maint: clean up _virDomainMemoryStat

2014-09-17 Thread Eric Blake
On 09/16/2014 07:19 AM, James wrote:
 I clean up all _virDomainMemoryStat.
 
 Signed-off-by: James james.wangyu...@huawei.com
 Signed-off-by: Wang Rui moon.wang...@huawei.com
 ---
  daemon/remote.c  | 2 +-
  src/driver.h | 2 +-
  src/lxc/lxc_driver.c | 2 +-
  src/qemu/qemu_driver.c   | 2 +-
  src/qemu/qemu_monitor_text.c | 2 +-
  src/remote/remote_driver.c   | 2 +-
  tools/virsh-domain-monitor.c | 2 +-
  7 files changed, 7 insertions(+), 7 deletions(-)

ACK; same story about the commit cleanup.  I'd also like to push a
.mailmap entry to consolidate your prior entries; am I correct that all
of these are from you?

$ git shortlog --author=james.wangyu...@huawei.com
James (1):
  util: virTimeFieldsThenRaw never returns negative

Wang Yufei (4):
  qemu: Avoid assigning unavailable migration ports
  docs: fix virDomainRestoreFlags description bug
  docs: fix double articles bug
  cgroup: Fix start VMs coincidently failed

Wangyufei (A) (1):
  docs: delete extra character

Wangyufei (James) (1):
  qemuAgentDispose: Reset lastError



 
 diff --git a/daemon/remote.c b/daemon/remote.c
 index 0ea2815..daa4b60 100644
 --- a/daemon/remote.c
 +++ b/daemon/remote.c
 @@ -1604,7 +1604,7 @@ remoteDispatchDomainMemoryStats(virNetServerPtr server 
 ATTRIBUTE_UNUSED,
  remote_domain_memory_stats_ret *ret)
  {
  virDomainPtr dom = NULL;
 -struct _virDomainMemoryStat *stats = NULL;
 +virDomainMemoryStatPtr stats = NULL;
  int nr_stats;
  size_t i;
  int rv = -1;
 diff --git a/src/driver.h b/src/driver.h
 index 76142bd..bb748c4 100644
 --- a/src/driver.h
 +++ b/src/driver.h
 @@ -535,7 +535,7 @@ typedef int
  
  typedef int
  (*virDrvDomainMemoryStats)(virDomainPtr domain,
 -   struct _virDomainMemoryStat *stats,
 +   virDomainMemoryStatPtr stats,
 unsigned int nr_stats,
 unsigned int flags);
  
 diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
 index 8ab4cf2..c3cd62c 100644
 --- a/src/lxc/lxc_driver.c
 +++ b/src/lxc/lxc_driver.c
 @@ -5387,7 +5387,7 @@ lxcNodeGetInfo(virConnectPtr conn,
  
  static int
  lxcDomainMemoryStats(virDomainPtr dom,
 - struct _virDomainMemoryStat *stats,
 + virDomainMemoryStatPtr stats,
   unsigned int nr_stats,
   unsigned int flags)
  {
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 9f5c977..59a4b3c 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -10178,7 +10178,7 @@ qemuDomainGetInterfaceParameters(virDomainPtr dom,
  
  static int
  qemuDomainMemoryStats(virDomainPtr dom,
 -  struct _virDomainMemoryStat *stats,
 +  virDomainMemoryStatPtr stats,
unsigned int nr_stats,
unsigned int flags)
  {
 diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
 index 2bc8261..46d2782 100644
 --- a/src/qemu/qemu_monitor_text.c
 +++ b/src/qemu/qemu_monitor_text.c
 @@ -684,7 +684,7 @@ int qemuMonitorTextGetBalloonInfo(qemuMonitorPtr mon,
  
  if ((offset = strstr(reply, BALLOON_PREFIX)) != NULL) {
  offset += strlen(BALLOON_PREFIX);
 -struct _virDomainMemoryStat stats[1];
 +virDomainMemoryStatStruct stats[1];
  
  if (qemuMonitorParseBalloonInfo(offset, stats, 1) == 0) {
  virReportError(VIR_ERR_INTERNAL_ERROR,
 diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
 index 4a1b04b..75a3a7b 100644
 --- a/src/remote/remote_driver.c
 +++ b/src/remote/remote_driver.c
 @@ -2690,7 +2690,7 @@ remoteDomainGetSchedulerType(virDomainPtr domain, int 
 *nparams)
  
  static int
  remoteDomainMemoryStats(virDomainPtr domain,
 -struct _virDomainMemoryStat *stats,
 +virDomainMemoryStatPtr stats,
  unsigned int nr_stats,
  unsigned int flags)
  {
 diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
 index b2e1fc8..4f6aaa3 100644
 --- a/tools/virsh-domain-monitor.c
 +++ b/tools/virsh-domain-monitor.c
 @@ -295,7 +295,7 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd)
  {
  virDomainPtr dom;
  const char *name;
 -struct _virDomainMemoryStat stats[VIR_DOMAIN_MEMORY_STAT_NR];
 +virDomainMemoryStatStruct stats[VIR_DOMAIN_MEMORY_STAT_NR];
  unsigned int nr_stats;
  size_t i;
  int ret = false;
 

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



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

[libvirt] [PATCH] qemu: Don't fail qemuProcessAttach for IOThreads if no JSON

2014-09-17 Thread John Ferlan
While doing some investigation for another bug I found that I could
not qemu-attach to the process and got the following:

   error: Operation not supported: JSON monitor is required

while running thru qemuProcessAttach. Since we can only get the data
using the JSON parser and if the guest to be attached to doesn't have
it we shouldn't just fail. See example in virsh qemu-attach for sample
command that failed.

Signed-off-by: John Ferlan jfer...@redhat.com
---

I also considered removing the call from qemuProcessAttach rather than
this approach.

 src/qemu/qemu_monitor.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 8927dbb..4342088 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4112,11 +4112,9 @@ qemuMonitorGetIOThreads(qemuMonitorPtr mon,
 return -1;
 }
 
-if (!mon-json) {
-virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
-   _(JSON monitor is required));
-return -1;
-}
+/* Requires JSON to make the query */
+if (!mon-json)
+return 0;
 
 return qemuMonitorJSONGetIOThreads(mon, iothreads);
 }
-- 
1.9.3

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


Re: [libvirt] [PATCH 6/6] qemu: Improve check for local storage

2014-09-17 Thread Peter Krempa
On 09/17/14 00:29, John Ferlan wrote:
 
 
 On 09/11/2014 01:47 PM, Peter Krempa wrote:
 Now that we have a simple function to check locality of storage, reuse
 it in qemuDomainCheckDiskPresence().

 Also reuse check for empty storage source.
 ---
  src/qemu/qemu_domain.c | 8 +++-
  1 file changed, 3 insertions(+), 5 deletions(-)

 
 ACK

Thanks, I've currently pushed 2/6 and 6/6 and I'll try to implement your
feedback on the rest later and I'll possibly repost the series if I find
the changes were not trivial.

 
 John
 

Peter



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

Re: [libvirt] [PATCH 0/3] Add rawio for hostdev

2014-09-17 Thread John Ferlan


On 09/09/2014 07:40 PM, John Ferlan wrote:
 The rawio setting for a device.../ disk.../ lun isn't duplicated
 in the scsi_host hostdev device for a lun.  Rather than requiring some
 disk in the disk list to add the rawio, add the rawio property to
 the scsi_host hostdev lun.
 
 John Ferlan (3):
   hostdev: Add rawio attribute to _virDomainHostdevSubsysSCSI
   qemu: Process the hostdev rawio setting
   domain_conf: Process the rawio for a hostdev device
 
  docs/formatdomain.html.in  | 12 ++--
  docs/schemas/domaincommon.rng  | 18 +++
  src/conf/domain_conf.c | 31 +++
  src/conf/domain_conf.h |  2 ++
  src/qemu/qemu_domain.c | 21 +
  src/qemu/qemu_domain.h |  4 +++
  src/qemu/qemu_driver.c |  1 +
  src/qemu/qemu_process.c| 20 -
  .../qemuxml2argv-hostdev-scsi-rawio.xml| 35 
 ++
  tests/qemuxml2xmltest.c|  1 +
  10 files changed, 135 insertions(+), 10 deletions(-)
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-rawio.xml
 

ping?

tks

John

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


Re: [libvirt] [PATCH] qemu: fix crash with shared disks

2014-09-17 Thread John Ferlan


On 09/17/2014 06:45 AM, Ján Tomko wrote:
 Commit f36a94f introduced a double free on all success paths
 in qemuSharedDeviceEntryInsert.
 
 Only call qemuSharedDeviceEntryFree on the error path and
 set entry to NULL before jumping there if the entry already
 is in the hash table.
 
 https://bugzilla.redhat.com/show_bug.cgi?id=1142722
 ---
  src/qemu/qemu_conf.c | 26 --
  1 file changed, 12 insertions(+), 14 deletions(-)
 
 diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
 index ac10b64..adc6caf 100644
 --- a/src/qemu/qemu_conf.c
 +++ b/src/qemu/qemu_conf.c
 @@ -1011,38 +1011,36 @@ qemuSharedDeviceEntryInsert(virQEMUDriverPtr driver,
  const char *name)
  {
  qemuSharedDeviceEntry *entry = NULL;
 -int ret = -1;
  
  if ((entry = virHashLookup(driver-sharedDevices, key))) {
  /* Nothing to do if the shared scsi host device is already
   * recorded in the table.
   */
 -if (qemuSharedDeviceEntryDomainExists(entry, name, NULL)) {
 -ret = 0;
 -goto cleanup;
 +if (!qemuSharedDeviceEntryDomainExists(entry, name, NULL)) {
 +if (VIR_EXPAND_N(entry-domains, entry-ref, 1)  0 ||
 +VIR_STRDUP(entry-domains[entry-ref - 1], name)  0) {
 +/* entry is owned by the hash table here */
 +entry = NULL;

[1] Assigning to NULL causes an issue

 +goto error;
 +}
  }
 -
 -if (VIR_EXPAND_N(entry-domains, entry-ref, 1)  0 ||
 -VIR_STRDUP(entry-domains[entry-ref - 1], name)  0)
 -goto cleanup;
  } else {
  if (VIR_ALLOC(entry)  0 ||
  VIR_ALLOC_N(entry-domains, 1)  0 ||
  VIR_STRDUP(entry-domains[0], name)  0)
 -goto cleanup;
 +goto error;
  
  entry-ref = 1;
  
  if (virHashAddEntry(driver-sharedDevices, key, entry))
 -goto cleanup;
 +goto error;
  }
  
 -ret = 0;
 +return 0;
  
 - cleanup:
 + error:
  qemuSharedDeviceEntryFree(entry, NULL);
[1]
Because this is prototyped as:

void qemuSharedDeviceEntryFree(void *payload, const void *name)
ATTRIBUTE_NONNULL(1);

Coverity gives us a warning when entry = NULL...

It's solveable by either allowing NULL for the function or only calling
if (entry)

ACK as long as we handle in some manner.

John

 -
 -return ret;
 +return -1;
  }
  
  
 

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


[libvirt] retiring v0.9.6-maint

2014-09-17 Thread Eric Blake
Any objections to retiring the v0.9.6-maint branch?  After all, we have
already retired the v0.9.11-maint branch
(http://libvirt.org/git/?p=libvirt.git;a=commit;h=cd0d348ed), and the
only activity on v0.9.6-maint since 0.9.6.4 was released in January 2013
was the backport of a single CVE fix.  The branch no longer builds
cleanly on Fedora 20, and while I could identify patches to backport to
fix the build situation, it's not worth my time if we can just retire
the branch.

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



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

[libvirt] [PATCH-RFC-V2] qemu: Add network bandwidth setting for ethernet interfaces

2014-09-17 Thread Anirban Chakraborty
V2:
Consolidate calls to virNetDevBandwidthSet
Clear bandwidth settings when the interface is detached or domain destroyed

V1:
Ethernet interfaces in libvirt currently do not support bandwidth setting.
For example, following xml file for an interface will not apply these
settings to corresponding qdiscs.

interface type=ethernet
  mac address=02:36:1d:18:2a:e4/
  model type=virtio/
  script path=/
  target dev=tap361d182a-e4/
  bandwidth
inbound average=984 peak=1024 burst=64/
outbound average=2000 peak=2048 burst=128/
  /bandwidth
/interface

Signed-off-by: Anirban Chakraborty abc...@juniper.net
---
 src/conf/domain_conf.h  |  7 +++
 src/lxc/lxc_process.c   | 27 ++-
 src/qemu/qemu_command.c |  9 -
 src/qemu/qemu_driver.c  | 21 +
 src/qemu/qemu_hotplug.c | 13 +
 src/util/virnetdevmacvlan.c | 10 --
 src/util/virnetdevmacvlan.h |  1 -
 7 files changed, 59 insertions(+), 29 deletions(-)

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 640a4c5..3c950f1 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -829,6 +829,13 @@ typedef enum {
 VIR_DOMAIN_NET_TYPE_LAST
 } virDomainNetType;

+/* check bandwidth configuration for a network interface */
+#define NETDEVIF_SUPPORT_BANDWIDTH(type) \
+   ((type == VIR_DOMAIN_NET_TYPE_ETHERNET || \
+ type == VIR_DOMAIN_NET_TYPE_NETWORK  || \
+ type == VIR_DOMAIN_NET_TYPE_BRIDGE   || \
+ type == VIR_DOMAIN_NET_TYPE_DIRECT) ? true : false)
+
 /* the backend driver used for virtio interfaces */
 typedef enum {
 VIR_DOMAIN_NET_BACKEND_TYPE_DEFAULT, /* prefer kernel, fall back to user */
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index ed30c37..5ef91e8 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -274,11 +274,6 @@ char *virLXCProcessSetupInterfaceBridged(virConnectPtr 
conn,
 if (virNetDevSetOnline(parentVeth, true)  0)
 goto cleanup;

-if (virNetDevBandwidthSet(net-ifname,
-  virDomainNetGetActualBandwidth(net),
-  false)  0)
-goto cleanup;
-
 if (net-filter 
 virDomainConfNWFilterInstantiate(conn, vm-uuid, net)  0)
 goto cleanup;
@@ -300,6 +295,7 @@ char *virLXCProcessSetupInterfaceDirect(virConnectPtr conn,
 virNetDevBandwidthPtr bw;
 virNetDevVPortProfilePtr prof;
 virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver);
+const char *linkdev = virDomainNetGetActualDirectDev(net);

 /* XXX how todo bandwidth controls ?
  * Since the 'net-ifname' is about to be moved to a different
@@ -329,16 +325,15 @@ char *virLXCProcessSetupInterfaceDirect(virConnectPtr 
conn,

 if (virNetDevMacVLanCreateWithVPortProfile(
 net-ifname, net-mac,
-virDomainNetGetActualDirectDev(net),
+linkdev,
 virDomainNetGetActualDirectMode(net),
 false, def-uuid,
-virDomainNetGetActualVirtPortProfile(net),
+prof,
 res_ifname,
 VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
 cfg-stateDir,
-virDomainNetGetActualBandwidth(net), 0)  0)
+0)  0)
 goto cleanup;
-
 ret = res_ifname;

  cleanup:
@@ -368,6 +363,7 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn,
 int ret = -1;
 size_t i;
 size_t niface = 0;
+int actualType;

 for (i = 0; i  def-nnets; i++) {
 char *veth = NULL;
@@ -381,7 +377,8 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn,
 if (VIR_EXPAND_N(*veths, *nveths, 1)  0)
 goto cleanup;

-switch (virDomainNetGetActualType(def-nets[i])) {
+actualType = virDomainNetGetActualType(def-nets[i]);
+switch (actualType) {
 case VIR_DOMAIN_NET_TYPE_NETWORK: {
 virNetworkPtr network;
 char *brname = NULL;
@@ -444,11 +441,15 @@ static int virLXCProcessSetupInterfaces(virConnectPtr 
conn,
 case VIR_DOMAIN_NET_TYPE_LAST:
 virReportError(VIR_ERR_INTERNAL_ERROR,
_(Unsupported network type %s),
-   virDomainNetTypeToString(
-   virDomainNetGetActualType(def-nets[i])
-   ));
+   virDomainNetTypeToString(actualType));
 goto cleanup;
 }
+/* set network bandwidth */
+if (NETDEVIF_SUPPORT_BANDWIDTH(actualType)  virNetDevBandwidthSet(
+def-nets[i]-ifname, virDomainNetGetActualBandwidth(
+def-nets[i]),
+false)  0)
+   goto cleanup;

 (*veths)[(*nveths)-1] = veth;

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f2e6e5a..404c51b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -191,7 +191,6 @@