Re: [libvirt] [PATCH] schemas: Allow all generic elements and attributes for all interfaces

2015-01-29 Thread Michal Privoznik
On 29.01.2015 11:25, Martin Kletzander wrote:
 On Wed, Jan 28, 2015 at 06:23:08PM +0100, Michal Privoznik wrote:
 There are some interface types (notably 'server' and 'client')
 which instead of allowing the default set of elements and
 attributes (like the rest do), try to enumerate only the elements
 they know of. This way it's, however, easy to miss something. For
 instance, the address/ element was not mentioned at all. This
 resulted in a strange behavior: when such interface was added
 into XML, the address was automatically generated by parsing
 code. Later, the formatted XML hasn't passed the RNG schema. This
 became more visible once we've turned on the XML validation on
 domain XML changes: appending an empty line at the end of
 formatted XML (to trick virsh think the XML had changed) made
 libvirt to refuse the very same XML it formatted.

 Instead of trying to find each element and attribute we are
 missing in the schema, lets just allow all the elements and
 attributes like we're doing that for the rest of types. It's no
 harm if the schema is wider than our parser allows.

 
 I'm pretty sure that separating it was the original intention, but
 until it's separated properly without breaking things, I agree with
 this fix.
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
 docs/schemas/domaincommon.rng  |  28 +---
 .../qemuxml2argv-interface-server.xml  | 157
 +
 
 So this file is added only for domainschematest?  Why not check it by
 xml2xml as well?

Good point.

 
 This effectively allows interface-options to be used with any
 interface, why not move it after the choice/ element, so we're not
 redundant?

Not a good point. I mean, I don't think one is allowed to:
element
  interlave
choice
  group/
  group/
choice
  /interleave
/element

simply because the choice groups may be defined by the value of an
attribute to the top parent element. Well, at least if I did it that way
I got an error from xmllint (and don't even get me started on it's error
reporting capabilities. For a long while I thought we have the worst
error reporting ever. Then I met xmllint).

 
 ACK if you fix those two things.

So I'm fixing the first issue and pushing. Thanks.

Michal

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


Re: [libvirt] [PATCH 2/7] qemu: Adjust EndAsyncJob for qemuDomainSaveInternal error path

2015-01-29 Thread John Ferlan


On 01/29/2015 08:43 AM, Martin Kletzander wrote:
 On Wed, Jan 28, 2015 at 05:25:01PM -0500, John Ferlan wrote:
 Commit id '540c339a' to fix issues with reference counting and transient
 domains moved the qemuDomainObjEndAsyncJob call prior to the attempt to
 restart the guest CPU's resulting in an error:

error: Failed to save domain rhel70 to /tmp/pl/rhel70.save
error: internal error: unexpected async job 3

 when (ret != 0) - eg, the error path from qemuDomainSaveMemory.

 This patch will adjust the logic to only call the EndAsyncJob after
 we've tried to restart the guest CPUs. It also needs to adjust the
 test for qemuDomainRemoveInactive to add the ret == 0 condition.

 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
 src/qemu/qemu_driver.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 190d472..0174c87 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -3208,7 +3208,6 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver,
 virDomainPtr dom,
  VIR_DOMAIN_EVENT_STOPPED,
  VIR_DOMAIN_EVENT_STOPPED_SAVED);
  endjob:
 -qemuDomainObjEndAsyncJob(driver, vm);
 if (ret != 0) {
 if (was_running  virDomainObjIsActive(vm)) {
 virErrorPtr save_err = virSaveLastError();
 @@ -3224,9 +3223,10 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver,
 virDomainPtr dom,
 virSetError(save_err);
 virFreeError(save_err);
 }
 -} else if (!vm-persistent) {
 -qemuDomainRemoveInactive(driver, vm);
 }
 +qemuDomainObjEndAsyncJob(driver, vm);
 +if (ret == 0  !vm-persistent)
 +qemuDomainRemoveInactive(driver, vm);

 
 We must definitely keep the job until we're done with changing things
 with the domain.  Have I really messed this up so much?  ACK whether
 you squash it in or not.
 

From my re-review of the referenced commit, I believe this was the only
function where endjob:/cleanup: processing needed the job still around
(and only because this command stops the CPU's and error recovery is
best effort).

I've combined the two patches into one, adjusted the commit message and
pushed just these two as one.

Tks -

John

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


Re: [libvirt] [PATCH] Remove flag checking in MacVLanCreate helper stub

2015-01-29 Thread Michal Privoznik
On 29.01.2015 10:07, Ján Tomko wrote:
 When compiling without WITH_MACVTAP, we can get:
 'unsupported flags (0x1) in function
 virNetDevMacVLanCreateWithVPortProfile'
 on an attempt to start a domain.
 
 Remove the flag check to reach the more helpful error:
 Cannot create macvlan devices on this platform
 
 https://bugzilla.redhat.com/show_bug.cgi?id=1186928
 ---
  src/util/virnetdevmacvlan.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)
 
 diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
 index f2eae06..6a8a923 100644
 --- a/src/util/virnetdevmacvlan.c
 +++ b/src/util/virnetdevmacvlan.c
 @@ -1066,9 +1066,8 @@ int virNetDevMacVLanCreateWithVPortProfile(const char 
 *ifname ATTRIBUTE_UNUSED,
 char **res_ifname 
 ATTRIBUTE_UNUSED,
 virNetDevVPortProfileOp vmop 
 ATTRIBUTE_UNUSED,
 char *stateDir ATTRIBUTE_UNUSED,
 -   unsigned int flags)
 +   unsigned int flags 
 ATTRIBUTE_UNUSED)
  {
 -virCheckFlags(0, -1);
  virReportSystemError(ENOSYS, %s,
   _(Cannot create macvlan devices on this 
 platform));
  return -1;
 

ACK

Michal

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


Re: [libvirt] [PATCH] virsh: man: Document behavior of some blkdeviotune's flags when querying

2015-01-29 Thread Martin Kletzander

On Thu, Jan 29, 2015 at 08:37:37AM +0100, Peter Krempa wrote:

--live and --config can't be specified together when querying the
configuration, but are valid when setting. The man page was hinting that
they are valid always.
---
tools/virsh.pod | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/virsh.pod b/tools/virsh.pod
index 1591341..804458e 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1100,9 +1100,10 @@ cannot be mixed with non-zero values for read or write.
If I--live is specified, affect a running guest.
If I--config is specified, affect the next boot of a persistent guest.
If I--current is specified, affect the current guest state.
-Both I--live and I--config flags may be given, but I--current is
-exclusive. If no flag is specified, behavior is different depending
-on hypervisor.
+When setting the disk io parameters both I--live and I--config flags may be
+given, but I--current is exclusive. For querying only one of I--live,
+I--config or I--current can be specified. If no flag is specified, behavior
+is different depending on hypervisor.

=item Bblockjob Idomain Ipath { [I--abort] [I--async] [I--pivot] |
[I--info] [I--raw] [I--bytes] | [Ibandwidth] }
--
2.2.2



ACK, did you check other similar commands use the same wording?


pgplT6T4DP4QW.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Remove flag checking in MacVLanCreate helper stub

2015-01-29 Thread Ján Tomko
When compiling without WITH_MACVTAP, we can get:
'unsupported flags (0x1) in function
virNetDevMacVLanCreateWithVPortProfile'
on an attempt to start a domain.

Remove the flag check to reach the more helpful error:
Cannot create macvlan devices on this platform

https://bugzilla.redhat.com/show_bug.cgi?id=1186928
---
 src/util/virnetdevmacvlan.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index f2eae06..6a8a923 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -1066,9 +1066,8 @@ int virNetDevMacVLanCreateWithVPortProfile(const char 
*ifname ATTRIBUTE_UNUSED,
char **res_ifname ATTRIBUTE_UNUSED,
virNetDevVPortProfileOp vmop 
ATTRIBUTE_UNUSED,
char *stateDir ATTRIBUTE_UNUSED,
-   unsigned int flags)
+   unsigned int flags ATTRIBUTE_UNUSED)
 {
-virCheckFlags(0, -1);
 virReportSystemError(ENOSYS, %s,
  _(Cannot create macvlan devices on this platform));
 return -1;
-- 
2.0.5

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


Re: [libvirt] [PATCH] virsh: man: Document behavior of some blkdeviotune's flags when querying

2015-01-29 Thread Peter Krempa
On Thu, Jan 29, 2015 at 09:18:03 +0100, Martin Kletzander wrote:
 On Thu, Jan 29, 2015 at 08:37:37AM +0100, Peter Krempa wrote:
 --live and --config can't be specified together when querying the
 configuration, but are valid when setting. The man page was hinting that
 they are valid always.
 ---
  tools/virsh.pod | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)
 

...


 ACK, did you check other similar commands use the same wording?

I was asked about this particular instance so I tweaked this one.

Pushed now, thanks.

Peter



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

Re: [libvirt] [PATCH] schemas: Allow all generic elements and attributes for all interfaces

2015-01-29 Thread Martin Kletzander

On Wed, Jan 28, 2015 at 06:23:08PM +0100, Michal Privoznik wrote:

There are some interface types (notably 'server' and 'client')
which instead of allowing the default set of elements and
attributes (like the rest do), try to enumerate only the elements
they know of. This way it's, however, easy to miss something. For
instance, the address/ element was not mentioned at all. This
resulted in a strange behavior: when such interface was added
into XML, the address was automatically generated by parsing
code. Later, the formatted XML hasn't passed the RNG schema. This
became more visible once we've turned on the XML validation on
domain XML changes: appending an empty line at the end of
formatted XML (to trick virsh think the XML had changed) made
libvirt to refuse the very same XML it formatted.

Instead of trying to find each element and attribute we are
missing in the schema, lets just allow all the elements and
attributes like we're doing that for the rest of types. It's no
harm if the schema is wider than our parser allows.



I'm pretty sure that separating it was the original intention, but
until it's separated properly without breaking things, I agree with
this fix.


Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
docs/schemas/domaincommon.rng  |  28 +---
.../qemuxml2argv-interface-server.xml  | 157 +


So this file is added only for domainschematest?  Why not check it by
xml2xml as well?

This effectively allows interface-options to be used with any
interface, why not move it after the choice/ element, so we're not
redundant?

ACK if you fix those two things.


pgpa7qnLJA32k.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2] conf: Disallow emulatorpin when numatune's in effect

2015-01-29 Thread Martin Kletzander

On Tue, Jan 27, 2015 at 01:37:26PM +, Daniel P. Berrange wrote:

On Tue, Jan 27, 2015 at 02:25:35PM +0100, Michal Privoznik wrote:

https://bugzilla.redhat.com/show_bug.cgi?id=1170492

In one of our previous commits (dc8b7ce7) we've done a functional
change even though it was intended as pure refactor. The problem is,
that the following XML:

  vcpu placement='static' current='2'6/vcpu
  cputune
emulatorpin cpuset='1-3'/
  /cputune

gets translated into this one:

  vcpu placement='auto' current='2'6/vcpu
  cputune
emulatorpin cpuset='1-3'/
  /cputune
  numatune
memory mode='strict' placement='auto'/
  /numatune

We should not change the vcpu placement mode.


In the subject you're talking about disallowing  emulatorpin but
here you're talking about not changing vcpu placement, so I'm a
little confused about what this patch is actually changing.

WRT $SUBJECT though, I don't see why emulatorpin should be
disallowed when numatune is present, as it is valid to pin
guest RAM to NUMA nodes and also set emulator pinnning.



I believe Michal just forgot to fix the subject after reworking the
code.



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


pgpKVK60wzF_s.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2] conf: Disallow emulatorpin when numatune's in effect

2015-01-29 Thread Martin Kletzander

On Tue, Jan 27, 2015 at 02:25:35PM +0100, Michal Privoznik wrote:

https://bugzilla.redhat.com/show_bug.cgi?id=1170492

In one of our previous commits (dc8b7ce7) we've done a functional
change even though it was intended as pure refactor. The problem is,
that the following XML:

 vcpu placement='static' current='2'6/vcpu
 cputune
   emulatorpin cpuset='1-3'/
 /cputune

gets translated into this one:

 vcpu placement='auto' current='2'6/vcpu
 cputune
   emulatorpin cpuset='1-3'/
 /cputune
 numatune
   memory mode='strict' placement='auto'/
 /numatune

We should not change the vcpu placement mode.



Tiny bit confusing, could you append the numatune/ element to the
before XML to make the commit message look like this?

... the following XML:

 vcpu placement='static' current='2'6/vcpu
 cputune
   emulatorpin cpuset='1-3'/
 /cputune
 numatune
   memory mode='strict' placement='auto'/
 /numatune

gets translated into this one:

 vcpu placement='auto' current='2'6/vcpu
 cputune
   emulatorpin cpuset='1-3'/
 /cputune
 numatune
   memory mode='strict' placement='auto'/
 /numatune
...
--

Maybe it's just me who finds it confusing.  Also make sure the commit
message is not from v1 since that patch had a different behaviour.

You also didn't change the test since v1, so please make sure it
corresponds to the XML in the commit message =)

That tests lead me to finding out (also thanks to you) that there is
one thing we are doing wrong.  Earlier, when vcpu placement was
'auto', we had to put pin emulator thread to the same pCPUs just
because the emulator thread creates the threads and we were lazy^W not
able to pin them properly.  However, since commit af2a1f05, we are not
limited by that and vCPUs can be placed automatically

I don't require you to fix that as well since that wasn't even your
first intention when sending this patch, but the tests should not be
testing this, but rather the thing you are fixing here (as wrote
above).

ACK with all those fixes (instead of the emulatorpin removal)
incorporated in, since this is a small change, but if you are eager to
send a v3, I can't prevent you doing that ;)


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

diff to v1:
-Martin's review worked in

src/conf/domain_conf.c |  3 +-
.../qemuxml2argv-cputune-numatune.xml  | 35 ++
.../qemuxml2xmlout-cputune-numatune.xml| 32 
tests/qemuxml2xmltest.c|  1 +
4 files changed, 70 insertions(+), 1 deletion(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-cputune-numatune.xml

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9ff3819..fbc0e96 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13173,7 +13173,8 @@ virDomainDefParseXML(xmlDocPtr xml,
  ctxt)  0)
goto error;

-if (virDomainNumatuneHasPlacementAuto(def-numatune)  !def-cpumask)
+if (virDomainNumatuneHasPlacementAuto(def-numatune) 
+!def-cpumask  !def-cputune.vcpupin  !def-cputune.emulatorpin)
def-placement_mode = VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO;

if ((n = virXPathNodeSet(./resource, ctxt, nodes))  0) {
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml
new file mode 100644
index 000..9759b48
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml
@@ -0,0 +1,35 @@
+domain type='kvm'
+  namedummy2/name
+  uuid4d92ec27-9ebf-400b-ae91-20c71c647c19/uuid
+  memory unit='KiB'131072/memory
+  currentMemory unit='KiB'65536/currentMemory
+  vcpu placement='auto' current='2'6/vcpu
+  cputune
+emulatorpin cpuset='1-3'/
+  /cputune
+  numatune
+memory mode='strict' placement='auto'/
+  /numatune
+  os
+type arch='x86_64' machine='pc-q35-2.3'hvm/type
+boot dev='hd'/
+  /os
+  clock offset='utc'/
+  on_poweroffdestroy/on_poweroff
+  on_rebootrestart/on_reboot
+  on_crashdestroy/on_crash
+  devices
+emulator/usr/bin/qemu-system-x86_64/emulator
+controller type='sata' index='0'
+  address type='pci' domain='0x' bus='0x00' slot='0x1f' 
function='0x2'/
+/controller
+controller type='pci' index='0' model='pcie-root'/
+controller type='pci' index='1' model='dmi-to-pci-bridge'
+  address type='pci' domain='0x' bus='0x00' slot='0x1e' 
function='0x0'/
+/controller
+controller type='pci' index='2' model='pci-bridge'
+  address type='pci' domain='0x' bus='0x01' slot='0x01' 
function='0x0'/
+/controller
+memballoon model='none'/
+  /devices
+/domain
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-cputune-numatune.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-cputune-numatune.xml
new file mode 100644
index 000..b33f57f
--- /dev/null
+++ 

Re: [libvirt] [PATCH v3 6/9] qemu: add functions for attach/detach RNG device via qemu monitor

2015-01-29 Thread Peter Krempa
On Sat, Jan 17, 2015 at 13:09:35 +0800, Luyao Huang wrote:
 qemuMonitorAttachRNGDev and qemuMonitorDetachRNGDev functions
 just do some basic check and then call qemuMonitorJSONAttachRNGDev
 and qemuMonitorDelObject to help us.
 
 Signed-off-by: Luyao Huang lhu...@redhat.com
 ---
  src/qemu/qemu_monitor.c  | 43 +++
  src/qemu/qemu_monitor.h  |  7 +++
  src/qemu/qemu_monitor_json.c | 43 +++
  src/qemu/qemu_monitor_json.h |  5 +
  4 files changed, 98 insertions(+)

...

 diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
 index da5c14d..33c3866 100644
 --- a/src/qemu/qemu_monitor_json.c
 +++ b/src/qemu/qemu_monitor_json.c
 @@ -6235,6 +6235,49 @@ qemuMonitorJSONDetachCharDev(qemuMonitorPtr mon,
  return ret;
  }
  
 +int
 +qemuMonitorJSONAttachRNGDev(qemuMonitorPtr mon,
 +const char *chrID,
 +const char *objID,
 +virDomainRNGDefPtr rng)
 +{
 +const char *type = NULL;
 +virJSONValuePtr props;
 +
 +if (!(props = virJSONValueNewObject()))
 +goto cleanup;
 +
 +switch ((virDomainRNGBackend) rng-backend) {
 +case VIR_DOMAIN_RNG_BACKEND_RANDOM:
 +type = rng-random;
 +if (virJSONValueObjectCreate(props, s:filename, rng-source.file, 
 NULL)  0)
 +goto cleanup;
 +break;
 +
 +case VIR_DOMAIN_RNG_BACKEND_EGD:
 +if (STRNEQ_NULLABLE(strstr(chrID, rng), strstr(objID, rng))) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(chardev id '%s' basic alias name is different 
 from '%s',
 +   chrID, objID));
 +goto cleanup;
 +}
 +type = rng-egd;
 +if (virJSONValueObjectCreate(props, s:chardev, chrID, NULL)  0)
 +goto cleanup;
 +break;
 +
 +case VIR_DOMAIN_RNG_BACKEND_LAST:
 +/*shouldn't happen*/
 +goto cleanup;
 +}
 +
 +return qemuMonitorJSONAddObject(mon, type, objID, props);
 +
 + cleanup:
 +virJSONValueFree(props);
 +return -1;
 +}

My recent series introduces a function that allows to unify the
commandline and monitor object creation so that we don't have code
duplication as would be introduced here.

As my series is not pushed yet and was posted after your series I'll
take over the series and refactor it to the new code to save you hassle
of re-doing it again and also refactoring the existing code.

Peter



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

[libvirt] Build failed in Jenkins: libvirt-syntax-check #3103

2015-01-29 Thread Jenkins CI
See http://honk.sigxcpu.org:8001/job/libvirt-syntax-check/3103/

--
Started by upstream project libvirt-build build number 3538
Building on master in workspace 
http://honk.sigxcpu.org:8001/job/libvirt-syntax-check/ws/
[workspace] $ /bin/sh -xe /tmp/hudson8816773861766117951.sh
+ make syntax-check
  GENbracket-spacing-check
GFDL_version
1.47 GFDL_version
TAB_in_indentation
0.52 TAB_in_indentation
Wundef_boolean
0.28 Wundef_boolean
avoid_attribute_unused_in_header
0.33 avoid_attribute_unused_in_header
avoid_ctype_macros
0.75 avoid_ctype_macros
avoid_if_before_free
6.51 avoid_if_before_free
avoid_strcase
0.66 avoid_strcase
avoid_write
0.73 avoid_write
bindtextdomain
0.43 bindtextdomain
cast_of_argument_to_free
0.73 cast_of_argument_to_free
cast_of_x_alloc_return_value
0.63 cast_of_x_alloc_return_value
changelog
0.29 changelog
const_long_option
0.69 const_long_option
copyright_check
0.80 copyright_check
copyright_format
1.84 copyright_format
copyright_usage
1.86 copyright_usage
correct_id_types
0.69 correct_id_types
curly_braces_style
1.36 curly_braces_style
error_message_period
0.61 error_message_period
error_message_warn_fatal
0.58 error_message_warn_fatal
flags_debug
1.29 flags_debug
flags_usage
src/util/virnetdevmacvlan.c:1069:   
unsigned int flags ATTRIBUTE_UNUSED)
maint.mk: flags should be checked with virCheckFlags
make: *** [sc_flags_usage] Error 1
Build step 'Execute shell' marked build as failure

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


Re: [libvirt] [PATCH] cpu: add Freescale ppc64 CPU models

2015-01-29 Thread Prerna Saxena

On Tuesday 27 January 2015 06:53 AM, hong-hua@freescale.com wrote:
 Hi Prerna,

 Currently, cpu_powerpc.c only support IBM ppc64 CPU models.
 Could your please help review the previous patch? With this patch, Freescale 
 ppc64 CPU modesl could also be recognized.

 # virsh cpu-models ppc64
 POWER7
 POWER7_v2.1
 POWER7_v2.3
 POWER7+_v2.1
 POWER8_v1.0
 POWERPC_e5500_v10
 POWERPC_e5500_v11
 POWERPC_e5500_v12
 POWERPC_e5500_v1020
 POWERPC_e6500_v10
 POWERPC_e6500_v20
 POWERPC_e6500_v120

 But PowerPC 32bit is still recognized as 'generic architecture'.
 # virsh cpu-models ppc
 error: failed to get CPU model names
 error: internal error: cannot find CPU map for generic architecture

 Is there any plan to support ppc 32bit CPU models in cpu_powerpc.c file?

Hi Olivia,
The current implementation only supports ppc64 architecture. Either the current 
driver needs to be extended for ppc 32 bit too, or new handler functions need 
to be implemented for 32 bit PowerPC.
I do not have a 32 bit PowerPC system to try this.

Patches are welcome, and I will be happy to help with reviews :-)

Regards,

-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India

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


Re: [libvirt] [PATCH v3 6/9] qemu: add functions for attach/detach RNG device via qemu monitor

2015-01-29 Thread Luyao Huang


On 01/29/2015 05:48 PM, Peter Krempa wrote:

On Sat, Jan 17, 2015 at 13:09:35 +0800, Luyao Huang wrote:

qemuMonitorAttachRNGDev and qemuMonitorDetachRNGDev functions
just do some basic check and then call qemuMonitorJSONAttachRNGDev
and qemuMonitorDelObject to help us.

Signed-off-by: Luyao Huang lhu...@redhat.com
---
  src/qemu/qemu_monitor.c  | 43 +++
  src/qemu/qemu_monitor.h  |  7 +++
  src/qemu/qemu_monitor_json.c | 43 +++
  src/qemu/qemu_monitor_json.h |  5 +
  4 files changed, 98 insertions(+)

...


diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index da5c14d..33c3866 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6235,6 +6235,49 @@ qemuMonitorJSONDetachCharDev(qemuMonitorPtr mon,
  return ret;
  }
  
+int

+qemuMonitorJSONAttachRNGDev(qemuMonitorPtr mon,
+const char *chrID,
+const char *objID,
+virDomainRNGDefPtr rng)
+{
+const char *type = NULL;
+virJSONValuePtr props;
+
+if (!(props = virJSONValueNewObject()))
+goto cleanup;
+
+switch ((virDomainRNGBackend) rng-backend) {
+case VIR_DOMAIN_RNG_BACKEND_RANDOM:
+type = rng-random;
+if (virJSONValueObjectCreate(props, s:filename, rng-source.file, 
NULL)  0)
+goto cleanup;
+break;
+
+case VIR_DOMAIN_RNG_BACKEND_EGD:
+if (STRNEQ_NULLABLE(strstr(chrID, rng), strstr(objID, rng))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(chardev id '%s' basic alias name is different from 
'%s',
+   chrID, objID));
+goto cleanup;
+}
+type = rng-egd;
+if (virJSONValueObjectCreate(props, s:chardev, chrID, NULL)  0)
+goto cleanup;
+break;
+
+case VIR_DOMAIN_RNG_BACKEND_LAST:
+/*shouldn't happen*/
+goto cleanup;
+}
+
+return qemuMonitorJSONAddObject(mon, type, objID, props);
+
+ cleanup:
+virJSONValueFree(props);
+return -1;
+}

My recent series introduces a function that allows to unify the
commandline and monitor object creation so that we don't have code
duplication as would be introduced here.


Yes, i notice that and your idea looks so cool to me ;)

As my series is not pushed yet and was posted after your series I'll
take over the series and refactor it to the new code to save you hassle
of re-doing it again and also refactoring the existing code.


Wow, thanks in advance for your help !

Peter



Luyao

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


Re: [libvirt] [PATCH 1/7] qemu: Save/restore error in error path for qemuDomainSaveInternal

2015-01-29 Thread Martin Kletzander

On Wed, Jan 28, 2015 at 05:25:00PM -0500, John Ferlan wrote:

If we get to endjob: because of some error earlier such as perhaps
failure from qemuDomainSaveMemory to open/create the save file and
the attempt to restart the CPU's fails, then we get the error from
that restart attempt displayed to the user rather than the error
from the failed attempt to create a save file.

Upstream commit id '540c339a' changed the flow of the code moving
the EndAsyncJob call and thus exposing the issue where an error in
restarting CPUs resulted in the following:

error: Failed to save domain rhel70 to /tmp/pl/rhel70.save
error: internal error: unexpected async job 3

where /tmp/pl is a NFS root squashed client where the failure to save
the file (EPERM or ENOTCONN) should result in a failure:

error: Failed to save domain rhel70 to /tmp/pl/rhel70.save
error: Error from child process creating '/tmp/pl/rhel70.save': Transport 
endpoint is not connected

or (REVISIT ORDER - that is if I move the virfile check for ENOTCONN
to sooner, then we'll never see ENOTCONN)

error: Failed to save domain rhel70 to /tmp/pl/rhel70.save
error: Error from child process creating '/tmp/pl/rhel70.save': Operation not 
permitted
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/qemu/qemu_driver.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1d3bee6..190d472 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3211,6 +3211,7 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, 
virDomainPtr dom,
qemuDomainObjEndAsyncJob(driver, vm);
if (ret != 0) {
if (was_running  virDomainObjIsActive(vm)) {
+virErrorPtr save_err = virSaveLastError();
rc = qemuProcessStartCPUs(driver, vm, dom-conn,
  VIR_DOMAIN_RUNNING_SAVE_CANCELED,
  QEMU_ASYNC_JOB_SAVE);
@@ -3220,6 +3221,8 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, 
virDomainPtr dom,
  
VIR_DOMAIN_EVENT_SUSPENDED,
  
VIR_DOMAIN_EVENT_SUSPENDED_API_ERROR);
}
+virSetError(save_err);
+virFreeError(save_err);
}
} else if (!vm-persistent) {
qemuDomainRemoveInactive(driver, vm);
--
2.1.0



Makes perfect sense, ACK.


pgpXCzlwkJ01E.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Fix syntax-check

2015-01-29 Thread Ján Tomko
My commit 08d1ae1 broke syntax-check by adding ATTRIBUTE_UNUSED
to the flags parameter.

Rename the parameter to unused_flags to bypass the check.
---
Pushed as a build fix.

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

diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index 6a8a923..72e1f65 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -1066,7 +1066,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char 
*ifname ATTRIBUTE_UNUSED,
char **res_ifname ATTRIBUTE_UNUSED,
virNetDevVPortProfileOp vmop 
ATTRIBUTE_UNUSED,
char *stateDir ATTRIBUTE_UNUSED,
-   unsigned int flags ATTRIBUTE_UNUSED)
+   unsigned int unused_flags 
ATTRIBUTE_UNUSED)
 {
 virReportSystemError(ENOSYS, %s,
  _(Cannot create macvlan devices on this platform));
-- 
2.0.5

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


Re: [libvirt] [PATCH] cpu: add Freescale ppc64 CPU models

2015-01-29 Thread Prerna Saxena

On Friday 23 January 2015 08:42 AM, Olivia Yin wrote:
 Signed-off-by: Olivia Yin hong-hua@freescale.com
 ---
  src/cpu/cpu_map.xml | 38 +-
  1 file changed, 37 insertions(+), 1 deletion(-)

 diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
 index bd9b056..c34874e 100644
 --- a/src/cpu/cpu_map.xml
 +++ b/src/cpu/cpu_map.xml
 @@ -1,4 +1,4 @@
 -cpus
 +ncpus

This looks to be a typo.

arch name='x86'
  !-- vendor definitions --
  vendor name='Intel' string='GenuineIntel'/
 @@ -600,6 +600,7 @@
arch name='ppc64'
  !-- vendor definitions --
  vendor name='IBM'/
 +vendor name='Freescale'/

  !-- IBM-based CPU models --
  model name='POWER7'
 @@ -657,5 +658,40 @@
pvr value='0x004d'/
  /model

 +!-- Freescale-based CPU models --
 +model name='POWERPC_e5500_v10'
 +  vendor name='Freescale'/
 +  pvr value='0x80240010'/
 +/model
 +
 +model name='POWERPC_e5500_v11'
 +  vendor name='Freescale'/
 +  pvr value='0x80240011'/
 +/model
 +
 +model name='POWERPC_e5500_v12'
 +  vendor name='Freescale'/
 +  pvr value='0x80240012'/
 +/model
 +
 + model name='POWERPC_e5500_v1020'
 +  vendor name='Freescale'/
 +  pvr value='0x80241020'/
 +/model
 +
 +model name='POWERPC_e6500_v10'
 +  vendor name='Freescale'/
 +  pvr value='0x80400010'/
 +/model
 +
 +model name='POWERPC_e6500_v20'
 +  vendor name='Freescale'/
 +  pvr value='0x80400020'/
 +/model
 +
 +model name='POWERPC_e6500_v120'
 +  vendor name='Freescale'/
 +  pvr value='0x80400120'/
 +/model
/arch
  /cpus
Rest of the patch looks good.

However, I had an observation. It appears that POWERPC_e6500_v10 , .._v20  
_v120 seem variants of the same processor family -- they share the same higher 
16 bits of PVR.

Do you need to specifically expose the different variants of these models in 
the management stack ?
Or the purpose of adding these entries is merely to enable libvirt to run on 
these boards ?

If the latter describes your need, you just need to add a generic entry for 
this family, such as :

+model name='POWERPC_e6500'
+  vendor name='Freescale'/
+  pvr value='0x8040'/
+/model

Libvirt driver for ppc64 currently has support to fallback to a generic entry 
for a given model if the exact PVR isnt found.

Regards,

-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India

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


Re: [libvirt] [PATCH 2/7] qemu: Adjust EndAsyncJob for qemuDomainSaveInternal error path

2015-01-29 Thread Martin Kletzander

On Wed, Jan 28, 2015 at 05:25:01PM -0500, John Ferlan wrote:

Commit id '540c339a' to fix issues with reference counting and transient
domains moved the qemuDomainObjEndAsyncJob call prior to the attempt to
restart the guest CPU's resulting in an error:

   error: Failed to save domain rhel70 to /tmp/pl/rhel70.save
   error: internal error: unexpected async job 3

when (ret != 0) - eg, the error path from qemuDomainSaveMemory.

This patch will adjust the logic to only call the EndAsyncJob after
we've tried to restart the guest CPUs. It also needs to adjust the
test for qemuDomainRemoveInactive to add the ret == 0 condition.

Signed-off-by: John Ferlan jfer...@redhat.com
---
src/qemu/qemu_driver.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 190d472..0174c87 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3208,7 +3208,6 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, 
virDomainPtr dom,
 VIR_DOMAIN_EVENT_STOPPED,
 VIR_DOMAIN_EVENT_STOPPED_SAVED);
 endjob:
-qemuDomainObjEndAsyncJob(driver, vm);
if (ret != 0) {
if (was_running  virDomainObjIsActive(vm)) {
virErrorPtr save_err = virSaveLastError();
@@ -3224,9 +3223,10 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, 
virDomainPtr dom,
virSetError(save_err);
virFreeError(save_err);
}
-} else if (!vm-persistent) {
-qemuDomainRemoveInactive(driver, vm);
}
+qemuDomainObjEndAsyncJob(driver, vm);
+if (ret == 0  !vm-persistent)
+qemuDomainRemoveInactive(driver, vm);



We must definitely keep the job until we're done with changing things
with the domain.  Have I really messed this up so much?  ACK whether
you squash it in or not.


 cleanup:
VIR_FREE(xml);
--
2.1.0

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


pgpidzMoYUu2p.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 11/12] qemu: command: Refactor NUMA backend object formatting to use JSON objs

2015-01-29 Thread Michal Privoznik
On 28.01.2015 11:30, Peter Krempa wrote:
 With the new JSON to argv formatter we are now able to represent the
 memory backend definitions in the JSON object format that is reusable
 for monitor use (hotplug) and then convert it into the shell string.
 This will avoid having two separate instances of the same code that
 would create the different formats.
 
 Previous refactors now allow to make this step without changes to the
 test suite.
 ---
  src/qemu/qemu_command.c | 109 
 +++-
  1 file changed, 62 insertions(+), 47 deletions(-)
 

So we will construct a JSON object just to produce a command line? Well,
I am afraid there's no better way if want to use the same code to
generate the monitor commands one day.

Michal

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


Re: [libvirt] [PATCH 06/12] qemu: command: Add helper to format -object strings from JSON representation

2015-01-29 Thread Michal Privoznik
On 28.01.2015 11:30, Peter Krempa wrote:
 Unlike -device, qemu uses a JSON object to add backend objects via the
 monitor rather than the string that would be passed on the commandline.
 
 To be able to reuse code parts that configure backends for various
 devices, this patch adds a helper that will allow generating the command
 line representations from the JSON property object.
 ---
  src/qemu/qemu_command.c | 111 +
  src/qemu/qemu_command.h |   4 ++
  tests/Makefile.am   |  13 -
  tests/qemucommandutiltest.c | 118 
 
  4 files changed, 244 insertions(+), 2 deletions(-)
  create mode 100644 tests/qemucommandutiltest.c
 
 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
 index 100deed..6f298ac 100644
 --- a/src/qemu/qemu_command.c
 +++ b/src/qemu/qemu_command.c
 @@ -416,6 +416,117 @@ qemuDomainSupportsNetdev(virDomainDefPtr def,
  return virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV);
  }
 
 +
 +static int
 +qemuBuildObjectCommandLinePropsInternal(const char *key,
 +const virJSONValue *value,
 +virBufferPtr buf,
 +bool nested)
 +{
 +virJSONValuePtr elem;
 +virBitmapPtr bitmap = NULL;
 +ssize_t pos = -1;
 +ssize_t end;
 +size_t i;
 +
 +switch ((virJSONType) value-type) {
 +case VIR_JSON_TYPE_STRING:
 +virBufferAsprintf(buf, ,%s=%s, key, value-data.string);
 +break;
 +
 +case VIR_JSON_TYPE_NUMBER:
 +virBufferAsprintf(buf, ,%s=%s, key, value-data.number);

So a number is a string? Me goes check the struct, and you're right. I
don't even ...

 +break;
 +
 +case VIR_JSON_TYPE_BOOLEAN:
 +if (value-data.boolean)
 +virBufferAsprintf(buf, ,%s=yes, key);
 +else
 +virBufferAsprintf(buf, ,%s=no, key);
 +
 +break;
 +
 +case VIR_JSON_TYPE_ARRAY:
 +if (nested) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(nested -object property arrays are not 
 supported));
 +return -1;
 +}
 +
 +if (virJSONValueGetArrayAsBitmap(value, bitmap) == 0) {
 +while ((pos = virBitmapNextSetBit(bitmap, pos))  -1) {
 +if ((end = virBitmapNextClearBit(bitmap, pos))  0)
 +end = virBitmapLastSetBit(bitmap) + 1;
 +
 +if (end - 1  pos) {
 +virBufferAsprintf(buf, ,%s=%zd-%zd, key, pos, end - 1);
 +pos = end;
 +} else {
 +virBufferAsprintf(buf, ,%s=%zd, key, pos);
 +}
 +}
 +} else {
 +/* fallback, treat the array as a non-bitmap, adding the key
 + * for each member */
 +for (i = 0; i  virJSONValueArraySize(value); i++) {
 +elem = virJSONValueArrayGet((virJSONValuePtr)value, i);
 +
 +/* recurse to avoid duplicating code */
 +if (qemuBuildObjectCommandLinePropsInternal(key, elem, buf,
 +true)  0)
 +return -1;
 +}
 +}
 +break;
 +
 +case VIR_JSON_TYPE_OBJECT:
 +case VIR_JSON_TYPE_NULL:
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(NULL and OBJECT JSON types can't be converted to 
 + commandline string));
 +return -1;
 +}
 +
 +virBitmapFree(bitmap);
 +return 0;
 +}
 +

Michal

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


Re: [libvirt] [PATCH 04/12] util: json: Add functions to convert JSON arrays from/to virBitmaps

2015-01-29 Thread Michal Privoznik
On 28.01.2015 11:30, Peter Krempa wrote:
 To be able to easily represent nodesets and other data stored in
 virBitmaps in libvirt, this patch introduces a set of helpers that allow
 to convert the bitmap to and from JSON value objects.
 ---
  src/libvirt_private.syms |   2 +
  src/util/virjson.c   | 113 
 +++
  src/util/virjson.h   |   4 ++
  3 files changed, 119 insertions(+)
 
 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
 index cc74e35..70c81a8 100644
 --- a/src/libvirt_private.syms
 +++ b/src/libvirt_private.syms
 @@ -1514,6 +1514,7 @@ virJSONValueArrayGet;
  virJSONValueArraySize;
  virJSONValueFree;
  virJSONValueFromString;
 +virJSONValueGetArrayAsBitmap;
  virJSONValueGetBoolean;
  virJSONValueGetNumberDouble;
  virJSONValueGetNumberInt;
 @@ -1523,6 +1524,7 @@ virJSONValueGetNumberUlong;
  virJSONValueGetString;
  virJSONValueIsNull;
  virJSONValueNewArray;
 +virJSONValueNewArrayFromBitmap;
  virJSONValueNewBoolean;
  virJSONValueNewNull;
  virJSONValueNewNumberDouble;
 diff --git a/src/util/virjson.c b/src/util/virjson.c
 index 9eb1bff..3e00650 100644
 --- a/src/util/virjson.c
 +++ b/src/util/virjson.c
 @@ -99,6 +99,8 @@ struct _virJSONParser {
   *
   * a: json object, must be non-NULL
   * A: json object, omitted if NULL
 + * m: a bitmap represented as a JSON array, must be non-NULL
 + * M: a bitmap represented as a JSON array, omitted if NULL
   *
   * The value corresponds to the selected type.
   *
 @@ -242,6 +244,28 @@ virJSONValueObjectAddVArgs(virJSONValuePtr obj,
  rc = virJSONValueObjectAppend(obj, key, val);
  }   break;
 
 +case 'M':
 +case 'm': {
 +virBitmapPtr map = va_arg(args, virBitmapPtr);
 +virJSONValuePtr jsonMap;
 +
 +if (!map) {
 +if (type == 'M')
 +continue;
 +
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(argument key '%s' must not have null 
 value),
 +   key);
 +goto cleanup;
 +}
 +
 +if (!(jsonMap = virJSONValueNewArrayFromBitmap(map)))
 +goto cleanup;
 +
 +if ((rc = virJSONValueObjectAppend(obj, key, jsonMap))  0)
 +virJSONValueFree(jsonMap);
 +} break;
 +
  default:
  virReportError(VIR_ERR_INTERNAL_ERROR,
 _(unsupported data type '%c' for arg '%s'), 
 type, key - 2);
 @@ -941,6 +965,95 @@ virJSONValueGetBoolean(virJSONValuePtr val,
  }
 
 
 +/**
 + * virJSONValueGetArrayAsBitmap:
 + * @val: JSON array to convert to bitmap
 + * @bitmap: New bitmap is allocated filled and returned via this argument
 + *
 + * Attempts a conversion of a JSON array to a bitmap. The members of the 
 array
 + * must be non-negative integers for the conversion to succeed. This function
 + * does not report libvirt errors (except for out-of-memory) so that it can 
 be
 + * used to probe that the array can be represented as a bitmap.
 + *
 + * Returns 0 on success and fills @bitmap; -1 on error and  @bitmap is set to
 + * NULL.
 + */
 +int
 +virJSONValueGetArrayAsBitmap(const virJSONValue *val,
 + virBitmapPtr *bitmap)
 +{
 +int ret = -1;
 +virJSONValuePtr elem;
 +size_t i;
 +unsigned long long *elems = NULL;
 +unsigned long long maxelem = 0;
 +
 +*bitmap = NULL;
 +
 +if (val-type != VIR_JSON_TYPE_ARRAY)
 +return -1;
 +
 +if (VIR_ALLOC_N(elems, val-data.array.nvalues)  0)
 +return -1;

This reports an error in case of failure ...

 +
 +/* first pass converts array members to numbers and finds the maximum */
 +for (i = 0; i  val-data.array.nvalues; i++) {
 +elem = val-data.array.values[i];
 +
 +if (elem-type != VIR_JSON_TYPE_NUMBER ||

while this does not. I'd rather make this report an error on all failures.

 +virStrToLong_ullp(elem-data.number, NULL, 10, elems[i])  0)
 +goto cleanup;
 +
 +if (elems[i]  maxelem)
 +maxelem = elems[i];
 +}
 +
 +if (!(*bitmap = virBitmapNew(maxelem + 1)))
 +goto cleanup;
 +
 +/* second pass sets the correct bits in the map */
 +for (i = 0; i  val-data.array.nvalues; i++)
 +ignore_value(virBitmapSetBit(*bitmap, elems[i]));
 +
 +ret = 0;
 +
 + cleanup:
 +VIR_FREE(elems);
 +
 +return ret;
 +}

Michal

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


Re: [libvirt] [PATCH 03/12] util: json: make value object creator universal by supporting adding

2015-01-29 Thread Michal Privoznik
On 28.01.2015 11:30, Peter Krempa wrote:
 To allow constructing of value objects stepwise explode the helper into
 separate steps and allow appending into existing value objects.
 ---
  src/libvirt_private.syms |  2 ++
  src/util/virjson.c   | 74 
 +++-
  src/util/virjson.h   |  5 
  3 files changed, 55 insertions(+), 26 deletions(-)
 
 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
 index 2bbce03..cc74e35 100644
 --- a/src/libvirt_private.syms
 +++ b/src/libvirt_private.syms
 @@ -1533,6 +1533,8 @@ virJSONValueNewNumberUlong;
  virJSONValueNewObject;
  virJSONValueNewString;
  virJSONValueNewStringLen;
 +virJSONValueObjectAdd;
 +virJSONValueObjectAddVArgs;
  virJSONValueObjectAppend;
  virJSONValueObjectAppendBoolean;
  virJSONValueObjectAppendNull;
 diff --git a/src/util/virjson.c b/src/util/virjson.c
 index 9f2e1cf..9eb1bff 100644
 --- a/src/util/virjson.c
 +++ b/src/util/virjson.c
 @@ -64,12 +64,11 @@ struct _virJSONParser {
 
 
  /**
 - * virJSONValueObjectCreateVArgs:
 - * @obj: returns the created JSON object
 - * @...: a key-value argument pairs, terminated by NULL
 + * virJSONValueObjectAddVArgs:
 + * @obj: JSON object to add the values to
 + * @args: a key-value argument pairs, terminated by NULL
   *
 - * Creates a JSON value object filled with key-value pairs supplied as 
 variable
 - * argument list.
 + * Adds the key-value pairs supplied as variable argument list to @obj.
   *
   * Keys look like   s:name  the first letter is a type code:
   * Explanation of type codes:
 @@ -104,22 +103,17 @@ struct _virJSONParser {
   * The value corresponds to the selected type.
   *
   * Returns -1 on error. 1 on success, if at least one key:pair was valid 0
 - * in case of no error but nothing was filled (@obj will be NULL).
 + * in case of no error but nothing was filled.
   */
  int
 -virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, va_list args)
 +virJSONValueObjectAddVArgs(virJSONValuePtr obj,
 +   va_list args)
  {
 -virJSONValuePtr jargs = NULL;
  char type;
  char *key;
  int ret = -1;
  int rc;
 
 -*obj = NULL;
 -
 -if (!(jargs = virJSONValueNewObject()))
 -goto cleanup;
 -
  while ((key = va_arg(args, char *)) != NULL) {
 
  if (strlen(key)  3) {
 @@ -146,7 +140,7 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, 
 va_list args)
 key);
  goto cleanup;
  }
 -rc = virJSONValueObjectAppendString(jargs, key, val);
 +rc = virJSONValueObjectAppendString(obj, key, val);
  }   break;
 
  case 'z':
 @@ -165,7 +159,7 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, 
 va_list args)
  if (!val  (type == 'z' || type == 'y'))
  continue;
 
 -rc = virJSONValueObjectAppendNumberInt(jargs, key, val);
 +rc = virJSONValueObjectAppendNumberInt(obj, key, val);
  }   break;
 
  case 'p':
 @@ -175,7 +169,7 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, 
 va_list args)
  if (!val  type == 'p')
  continue;
 
 -rc = virJSONValueObjectAppendNumberUint(jargs, key, val);
 +rc = virJSONValueObjectAppendNumberUint(obj, key, val);
  }   break;
 
  case 'Z':
 @@ -194,7 +188,7 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, 
 va_list args)
  if (!val  (type == 'Z' || type == 'Y'))
  continue;
 
 -rc = virJSONValueObjectAppendNumberLong(jargs, key, val);
 +rc = virJSONValueObjectAppendNumberLong(obj, key, val);
  }   break;
 
  case 'P':
 @@ -209,12 +203,12 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, 
 va_list args)
  if (!val  type == 'P')
  continue;
 
 -rc = virJSONValueObjectAppendNumberLong(jargs, key, val);
 +rc = virJSONValueObjectAppendNumberLong(obj, key, val);
  }   break;
 
  case 'd': {
  double val = va_arg(args, double);
 -rc = virJSONValueObjectAppendNumberDouble(jargs, key, val);
 +rc = virJSONValueObjectAppendNumberDouble(obj, key, val);
  }   break;
 
  case 'B':
 @@ -224,11 +218,11 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, 
 va_list args)
  if (!val  type == 'B')
  continue;
 
 -rc = virJSONValueObjectAppendBoolean(jargs, key, val);
 +rc = virJSONValueObjectAppendBoolean(obj, key, val);
  }   break;
 
  case 'n': {
 -rc = virJSONValueObjectAppendNull(jargs, key);
 +rc = virJSONValueObjectAppendNull(obj, key);
  }   break;
 
  case 'A':
 @@ -245,7 +239,7 @@ virJSONValueObjectCreateVArgs(virJSONValuePtr *obj, 
 va_list args)
  goto cleanup;
  }
 
 -rc = 

Re: [libvirt] [PATCH 11/12] qemu: command: Refactor NUMA backend object formatting to use JSON objs

2015-01-29 Thread John Ferlan


On 01/28/2015 05:30 AM, Peter Krempa wrote:
 With the new JSON to argv formatter we are now able to represent the
 memory backend definitions in the JSON object format that is reusable
 for monitor use (hotplug) and then convert it into the shell string.
 This will avoid having two separate instances of the same code that
 would create the different formats.
 
 Previous refactors now allow to make this step without changes to the
 test suite.
 ---
  src/qemu/qemu_command.c | 109 
 +++-
  1 file changed, 62 insertions(+), 47 deletions(-)
 

Ran changes through my Coverity checker... Issue found with this change...

 diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
 index 0c343b6..bb58a09 100644
 --- a/src/qemu/qemu_command.c
 +++ b/src/qemu/qemu_command.c
 @@ -4533,12 +4533,10 @@ qemuBuildMemoryBackendStr(unsigned long long size,
virDomainDefPtr def,
virQEMUCapsPtr qemuCaps,
virQEMUDriverConfigPtr cfg,
 -  const char *aliasPrefix,
 -  size_t id,
 -  char **backendStr,
 +  const char **backendType,
 +  virJSONValuePtr *backendProps,
bool force)
  {
 -virBuffer buf = VIR_BUFFER_INITIALIZER;
  virDomainHugePagePtr master_hugepage = NULL;
  virDomainHugePagePtr hugepage = NULL;
  virDomainNumatuneMemMode mode;
 @@ -4546,11 +4544,15 @@ qemuBuildMemoryBackendStr(unsigned long long size,
  virMemAccess memAccess = def-cpu-cells[guestNode].memAccess;
  size_t i;
  char *mem_path = NULL;
 -char *nodemask = NULL;
 -char *tmpmask = NULL, *next = NULL;
 +virBitmapPtr nodemask = NULL;
  int ret = -1;
 +virJSONValuePtr props = NULL;
 
 -*backendStr = NULL;
 +*backendProps = NULL;
 +*backendType = NULL;
 +
 +if (!(props = virJSONValueNewObject()))
 +return -1;
 
  mode = virDomainNumatuneGetMode(def-numatune, guestNode);
 
 @@ -4623,16 +4625,23 @@ qemuBuildMemoryBackendStr(unsigned long long size,
  if (!(mem_path = qemuGetHugepagePath(cfg-hugetlbfs[i])))
  goto cleanup;
 
 -virBufferAsprintf(buf, memory-backend-file,id=%s%zu, aliasPrefix, 
 id);
 -virBufferAsprintf(buf, ,prealloc=yes,mem-path=%s, mem_path);
 +*backendType = memory-backend-file;
 +
 +if (virJSONValueObjectAdd(props,
 +  b:prealloc, true,
 +  s:mem-path, mem_path,
 +  NULL)  0)
 +goto cleanup;
 
  switch (memAccess) {
  case VIR_MEM_ACCESS_SHARED:
 -virBufferAddLit(buf, ,share=yes);
 +if (virJSONValueObjectAdd(props, b:share, true, NULL)  0)
 +goto cleanup;
  break;
 
  case VIR_MEM_ACCESS_PRIVATE:
 -virBufferAddLit(buf, ,share=no);
 +if (virJSONValueObjectAdd(props, b:share, false, NULL)  0)
 +goto cleanup;
  break;
 
  case VIR_MEM_ACCESS_DEFAULT:
 @@ -4647,43 +4656,30 @@ qemuBuildMemoryBackendStr(unsigned long long size,
  goto cleanup;
  }
 
 -virBufferAsprintf(buf, memory-backend-ram,id=%s%zu, aliasPrefix, 
 id);
 +*backendType = memory-backend-ram;
  }
 
 -virBufferAsprintf(buf, ,size=%llu, size * 1024);
 +if (virJSONValueObjectAdd(props, U:size, size * 1024, NULL)  0)
 +goto cleanup;
 
  if (userNodeset) {
 -if (!(nodemask = virBitmapFormat(userNodeset)))
 -goto cleanup;
 +nodemask = userNodeset;
  } else {
 -if (virDomainNumatuneMaybeFormatNodeset(def-numatune, autoNodeset,
 -nodemask, guestNode)  0)
 +if (virDomainNumatuneMaybeGetNodeset(def-numatune, autoNodeset,
 + nodemask, guestNode)  0)
  goto cleanup;
  }
 
  if (nodemask) {
 -if (strchr(nodemask, ',') 
 -!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NUMA)) {
 -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
 -   _(disjoint NUMA node ranges are not supported 
 - with this QEMU));
 +if (virJSONValueObjectAdd(props,
 +  m:host-nodes, nodemask,
 +  S:policy, 
 qemuNumaPolicyTypeToString(mode),
 +  NULL)  0)
  goto cleanup;
 -}
 -
 -for (tmpmask = nodemask; tmpmask; tmpmask = next) {
 -if ((next = strchr(tmpmask, ',')))
 -*(next++) = '\0';
 -virBufferAddLit(buf, ,host-nodes=);
 -virBufferAdd(buf, tmpmask, -1);
 -}
 -
 -virBufferAsprintf(buf, ,policy=%s,  
 

[libvirt] Jenkins build is back to normal : libvirt-syntax-check #3104

2015-01-29 Thread Jenkins CI
See http://honk.sigxcpu.org:8001/job/libvirt-syntax-check/3104/

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


[libvirt] [PATCH v3] conf: Don't mangle vcpu placement randomly

2015-01-29 Thread Michal Privoznik
https://bugzilla.redhat.com/show_bug.cgi?id=1170492

In one of our previous commits (dc8b7ce7) we've done a functional
change even though it was intended as pure refactor. The problem is,
that the following XML:

 vcpu placement='static' current='2'6/vcpu
 cputune
   emulatorpin cpuset='1-3'/
 /cputune
 numatune
   memory mode='strict' placement='auto'/
 /numatune

gets translated into this one:

 vcpu placement='auto' current='2'6/vcpu
 cputune
   emulatorpin cpuset='1-3'/
 /cputune
 numatune
   memory mode='strict' placement='auto'/
 /numatune

We should not change the vcpu placement mode. Moreover, we're doing
something similar in case of emulatorpin and iothreadpin. If they were
set, but vcpu placement was auto, we've mistakenly removed them from
the domain XML even though we are able to set them independently on
vcpus.

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

diff to v2:
-Martin's review worked in

 src/conf/domain_conf.c | 84 +-
 .../qemuxml2argv-cputune-numatune.xml  | 35 +
 tests/qemuxml2xmltest.c|  1 +
 3 files changed, 70 insertions(+), 50 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cputune-numatune.xml

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d562e1a..706e5d2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13084,28 +13084,20 @@ virDomainDefParseXML(xmlDocPtr xml,
 goto error;
 }
 
-/* Ignore emulatorpin if vcpu placement is auto, they
- * conflicts with each other, and vcpu placement can't be
- * simply ignored, as numatune's placement defaults to it.
- */
 if (n) {
-if (def-placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) {
-if (n  1) {
-virReportError(VIR_ERR_XML_ERROR, %s,
-   _(only one emulatorpin is supported));
-VIR_FREE(nodes);
-goto error;
-}
-
-def-cputune.emulatorpin = virDomainVcpuPinDefParseXML(nodes[0],
-   ctxt, 0,
-   true, 
false);
-
-if (!def-cputune.emulatorpin)
-goto error;
-} else {
-VIR_WARN(Ignore emulatorpin for vcpu placement is 'auto');
+if (n  1) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+   _(only one emulatorpin is supported));
+VIR_FREE(nodes);
+goto error;
 }
+
+def-cputune.emulatorpin = virDomainVcpuPinDefParseXML(nodes[0],
+   ctxt, 0,
+   true, false);
+
+if (!def-cputune.emulatorpin)
+goto error;
 }
 VIR_FREE(nodes);
 
@@ -13116,38 +13108,28 @@ virDomainDefParseXML(xmlDocPtr xml,
 goto error;
 }
 
-/* Ignore iothreadpin if vcpu placement is auto, they
- * conflict with each other, and vcpu placement can't be
- * simply ignored, as numatune's placement defaults to it.
- */
-if (n) {
-if (def-placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO) {
-if (VIR_ALLOC_N(def-cputune.iothreadspin, n)  0)
-goto error;
+if (n  VIR_ALLOC_N(def-cputune.iothreadspin, n)  0)
+goto error;
 
-for (i = 0; i  n; i++) {
-virDomainVcpuPinDefPtr iothreadpin = NULL;
-iothreadpin = virDomainVcpuPinDefParseXML(nodes[i], ctxt,
-  def-iothreads,
-  false, true);
-if (!iothreadpin)
-goto error;
+for (i = 0; i  n; i++) {
+virDomainVcpuPinDefPtr iothreadpin = NULL;
+iothreadpin = virDomainVcpuPinDefParseXML(nodes[i], ctxt,
+  def-iothreads,
+  false, true);
+if (!iothreadpin)
+goto error;
 
-if (virDomainVcpuPinIsDuplicate(def-cputune.iothreadspin,
-def-cputune.niothreadspin,
-iothreadpin-vcpuid)) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   _(duplicate iothreadpin for same 
iothread));
-virDomainVcpuPinDefFree(iothreadpin);
-goto error;
-}
-
-def-cputune.iothreadspin[def-cputune.niothreadspin++] =
-iothreadpin;
-}
-} else {
-VIR_WARN(Ignore iothreadpin for vcpu placement is 'auto');
+if (virDomainVcpuPinIsDuplicate(def-cputune.iothreadspin,
+  

Re: [libvirt] [PATCH v5 1/2] qemu: Allow UEFI paths to be specified at compile time

2015-01-29 Thread Martin Kletzander

On Mon, Jan 26, 2015 at 03:38:04PM +0100, Michal Privoznik wrote:

Up until now there are just two ways how to specify UEFI paths to
libvirt. The first one is editing qemu.conf, the other is editing
qemu_conf.c and recompile which is not that fancy. So, new
configure option is introduced: --with-loader-nvram which takes a
list of pairs of UEFI firmware and NVRAM store. This way, the
compiled in defaults can be passed during compile time without
need to change the code itself.

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
configure.ac   | 17 +
src/qemu/qemu_conf.c   | 49 +
tests/domaincapstest.c | 15 ---
3 files changed, 74 insertions(+), 7 deletions(-)

diff --git a/configure.ac b/configure.ac
index f370475..b121777 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2789,6 +2789,23 @@ AC_ARG_WITH([default-editor],
  [DEFAULT_EDITOR=vi])
AC_DEFINE_UNQUOTED([DEFAULT_EDITOR], [$DEFAULT_EDITOR], [Default editor to 
use])

+AC_ARG_WITH([loader-nvram],
+  [AS_HELP_STRING([--with-loader-nvram],
+[Pass list of pairs of loader:nvram paths. Both
+ pairs and list items are separated by a colon.
+ @:default=paths to OVMF and its clones@:@])],
+ [if test $withval = no; then
+withval=
+  else
+l=`echo $withval | tr ':' '\n' | wc -l`
+if test $((l % 2)) -ne 0; then


Sorry for the confusion I caused to you, but I'd rather you use `expr
$l % 2`.

ACK with that changed, my ACK to 2/2 still stands.


pgpwAUB2LdMew.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 02/12] test: utils: Add helpers for automatic numbering of test cases

2015-01-29 Thread John Ferlan

I don't see this being used anywhere in this set of patches...  I assume
you have some other upcoming patch series that will use it...

NITs

On 01/28/2015 05:30 AM, Peter Krempa wrote:
 Adding or reordering test cases is usually a pain due to static test
 case names that are then passed to virtTestRun(). To ease the numbering
 of test cases, this patch adds two simple helpers that generate the test
 names according to the order they are run. The test name can be
 configured via the reset function.
 
 This will allow us to freely add test cases in mid of test groups

s/in mid of/in the middle of/

 without the need to re-number the rest of test cases.
 ---
  tests/testutils.c | 46 ++
  tests/testutils.h |  3 +++
  2 files changed, 49 insertions(+)
 
 diff --git a/tests/testutils.c b/tests/testutils.c
 index 9a79f98..c7d2615 100644
 --- a/tests/testutils.c
 +++ b/tests/testutils.c
 @@ -986,3 +986,49 @@ virDomainXMLOptionPtr 
 virTestGenericDomainXMLConfInit(void)
   virTestGenericPrivateDataCallbacks,
   NULL);
  }
 +
 +
 +static int virtTestCounter;
 +static char virtTestCounterStr[128];
 +static char *virtTestCounterPrefixEndOffset;
 +
 +
 +/**
 + * virtTestCounterReset:
 + * @prefix: name of the test group
 + *
 + * Resets the counter and sets up the test group name to use with
 + * virtTestCounterNext(). This function is not thread safe.
 + */
 +void
 +virtTestCounterReset(const char *prefix)
 +{
 +virtTestCounter = 0;

Not that it'd happen, but if the prefix was larger than 128
characters... ;-)

 +
 +ignore_value(virStrcpyStatic(virtTestCounterStr, prefix));
 +virtTestCounterPrefixEndOffset = strchrnul(virtTestCounterStr, '\0');
 +}
 +
 +
 +/**
 + * virtTestCounterNext:
 + *
 + * This function is designed to ease test creation and reordering by adding
 + * a way to do automagic test case numbring.

s/numbring/numbering

 + *
 + * Returns string consisting of test name prefix configured via
 + * virtTestCounterReset() and a number that increments in every call of this
 + * function. This function is not thread safe.
 + */
 +const char
 +*virtTestCounterNext(void)
 +{
 +size_t len = ARRAY_CARDINALITY(virtTestCounterStr);
 +
 +/* calculate length of the rest of the string */
 +len -= (virtTestCounterPrefixEndOffset - virtTestCounterStr);
 +
 +snprintf(virtTestCounterPrefixEndOffset, len, %d, ++virtTestCounter);
 +
 +return virtTestCounterStr;
 +}
 diff --git a/tests/testutils.h b/tests/testutils.h
 index d78818d..155b30f 100644
 --- a/tests/testutils.h
 +++ b/tests/testutils.h
 @@ -82,6 +82,9 @@ char *virtTestLogContentAndReset(void);
 
  void virtTestQuiesceLibvirtErrors(bool always);
 
 +void virtTestCounterReset(const char *prefix);
 +const char *virtTestCounterNext(void);
 +
  int virtTestMain(int argc,
   char **argv,
   int (*func)(void));
 

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


Re: [libvirt] [PATCH 05/12] util: json: add helper to iterate JSON object key=value pairs

2015-01-29 Thread Eric Blake
On 01/28/2015 03:30 AM, Peter Krempa wrote:
 This helper easies iterating all key=value pairs stored in a JSON

s/easies/eases/

 object. Usually we pick only certain known keys from a JSON object, but
 this will allow to walk complete objects and have the callback act on
 those.
 ---
  src/libvirt_private.syms |  1 +
  src/util/virjson.c   | 33 +
  src/util/virjson.h   |  8 
  3 files changed, 42 insertions(+)
 

Sounds useful.  ACK.

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



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

Re: [libvirt] [PATCH 02/12] test: utils: Add helpers for automatic numbering of test cases

2015-01-29 Thread John Ferlan


On 01/29/2015 02:23 PM, John Ferlan wrote:
 
 I don't see this being used anywhere in this set of patches...  I assume
 you have some other upcoming patch series that will use it...
 

NM - my cscope autorebuild failed sigh... I found the usage in 6/12

John

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


Re: [libvirt] [PATCH 4/7] virfile: Adjust error path for virFileOpenForked

2015-01-29 Thread Eric Blake
On 01/28/2015 03:25 PM, John Ferlan wrote:
 Rather than have a dummy waitpid loop and return of the failure status
 from recvfd, adjust the logic to save the recvfd error  fd and then
 in priority order:
 
 if waitpid failed, use that
 if waitpid succeeded, but the child reported failure, use that
 regardless of recvfd status
 if waitpid succeeded and reported success, but recvfd failed with
 anything other than EACCES or ENOTCONN, use recvfd's result
 otherwise, waitpid and recvfd succeeded, return the fd
 
 NOTE: Original logic to retry the open and force owner mode was
 documented as only being attempted if we had already tried opening
 with the fork+setuid, but checked flags vs. VIR_FILE_OPEN_NOFORK which
 is counter to how we would get to that point. So that code was removed.
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/util/virfile.c | 67 
 +++---
  1 file changed, 39 insertions(+), 28 deletions(-)
 

 +/*
 + * if waitpid succeeded, but the child reported failure, use that
 + * regardless of recvfd status
 + */
 +if (WIFEXITED(status)  WEXITSTATUS(status) != 0) {
 +ret = WEXITSTATUS(status);
 +virReportSystemError(ret,

waitpid reporting !WIFEXITED(status) is a case of the child reporting
failure (well, reporting abnormal exit due to signal), so it might
simplify things if we just do[1]:

if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {

except that then you have to distinguish between abnormal exit vs. exit
encoding a known errno value when constructing the error message.



 +
 +/* if waitpid succeeded and reported success, but recvfd failed with
 + * anything other than EACCES or ENOTCONN, use recvfd's result
 + */
 +if (WIFEXITED(status)  WEXITSTATUS(status) == 0  fd  0 
 +!(recvfd_errno == EACCES || recvfd_errno == ENOTCONN)) {
 +virReportSystemError(recvfd_errno,
 + _(failed recvfd for child creating '%s'),
 + path);
 +return -recvfd_errno;
 +}

Hmm. It may be sufficient to just treat ALL recvfd failures as the exit
status if we get here, since if recvfd failed, then fd == -1...

 +
 +/* Some sort of abnormal child termination */
 +if (!WIFEXITED(status) || fd == -1) {
 +VIR_FORCE_CLOSE(fd);
 +virReportError(VIR_ERR_INTERNAL_ERROR,
 +   _(child exited abnormally, failed to create '%s'),
 +   path);
 +return -EACCES;

...and we would be failing anyways, but losing the errno value from
recvfd.  And if you change point [1] above to catch abnormal exit, then
this entire if branch becomes dead.

So how about:

- if waitpid failed, use that errno value
- waitpid succeeded, but if the child exited abnormally, report failure
(not sure what errno value to use here)
- waitpid succeeded, but if the child reported non-zero status, report
failure (use the errno value that the child encoded into exit status)
- waitpid succeeded, but if recvfd failed, report recvfd_errno
- waitpid and recvfd succeeded, use the fd

Sorry for making you churn on this, given my first-round off-list
thoughts on the matter, but hopefully it turns out a little easier to read.

-- 
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 0/7] Resolve issues saving domain to NFS root squashed client

2015-01-29 Thread John Ferlan


On 01/28/2015 05:24 PM, John Ferlan wrote:
 Lots of details here:

...snip...

 
 
 John Ferlan (7):
   qemu: Save/restore error in error path for qemuDomainSaveInternal
   qemu: Adjust EndAsyncJob for qemuDomainSaveInternal error path
   virfile: Need to check for ENOTCONN from recvfd failure
   virfile: Adjust error path for virFileOpenForked
   qemu: Don't unconditionally delete file in qemuOpenFileAs
   qemu: remove failed create prior to root squash share path
   qemu: qemuOpenFileAs - set flag VIR_FILE_OPEN_FORCE_MODE
 
  src/qemu/qemu_driver.c | 24 +++---
  src/util/virfile.c | 66 
 +-
  2 files changed, 60 insertions(+), 30 deletions(-)
 

Thanks for the reviews so far...

  I combined patches 1  2, changed the commit text and pushed

  I combined patches 5  6, changed the commit text and pushed along
with patch 3

  That leaves the refactor of virFileOpenForked (patch 4) and setting
VIR_FILE_OPEN_FORCE_MODE (patch 7) as things that will show up in a v2.

John

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


Re: [libvirt] [PATCH 7/7] qemu: qemuOpenFileAs - set flag VIR_FILE_OPEN_FORCE_MODE

2015-01-29 Thread Eric Blake
On 01/28/2015 03:25 PM, John Ferlan wrote:
 In the event we're falling into the code that tries to create the file
 in a forked environment (VIR_FILE_OPEN_FORK) we pass different mode bits,
 but those are never set because the virFileOpenForceOwnerMode has a check
 if the OPEN_FORCE_MODE bit is set before attempting to change the mode.
 
 Since this is a special case it seems reasonable to set u+rw,g+rw,o
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/qemu/qemu_driver.c | 3 +++
  1 file changed, 3 insertions(+)

I'd feel better with more testing before we take this one; Laine is more
familiar with testing root-squash setups.

 
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 91fefa9..651f84b 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -2965,6 +2965,9 @@ qemuOpenFileAs(uid_t fallback_uid, gid_t fallback_gid,
  
  /* Retry creating the file as qemu user */
  
 +/* Since we're passing different modes... */
 +vfoflags |= VIR_FILE_OPEN_FORCE_MODE;
 +
  if ((fd = virFileOpenAs(path, oflags,
  S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP,
  fallback_uid, fallback_gid,
 

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



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

Re: [libvirt] [PATCH 1/7] qemu: Save/restore error in error path for qemuDomainSaveInternal

2015-01-29 Thread John Ferlan


On 01/29/2015 03:13 PM, Eric Blake wrote:
 On 01/28/2015 03:25 PM, John Ferlan wrote:
 If we get to endjob: because of some error earlier such as perhaps
 failure from qemuDomainSaveMemory to open/create the save file and
 the attempt to restart the CPU's fails, then we get the error from
 that restart attempt displayed to the user rather than the error
 from the failed attempt to create a save file.

 Upstream commit id '540c339a' changed the flow of the code moving
 the EndAsyncJob call and thus exposing the issue where an error in
 restarting CPUs resulted in the following:

 error: Failed to save domain rhel70 to /tmp/pl/rhel70.save
 error: internal error: unexpected async job 3

 where /tmp/pl is a NFS root squashed client where the failure to save
 the file (EPERM or ENOTCONN) should result in a failure:

 error: Failed to save domain rhel70 to /tmp/pl/rhel70.save
 error: Error from child process creating '/tmp/pl/rhel70.save': Transport 
 endpoint is not connected

 or (REVISIT ORDER - that is if I move the virfile check for ENOTCONN
 to sooner, then we'll never see ENOTCONN)
 
 Is this a note about the possibility of applying the series in a
 different order?
 

Yeah - it was... But when I combined 12 after mkletzan's ACK - I
removed it...

John

 error: Failed to save domain rhel70 to /tmp/pl/rhel70.save
 error: Error from child process creating '/tmp/pl/rhel70.save': Operation 
 not permitted
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/qemu/qemu_driver.c | 3 +++
  1 file changed, 3 insertions(+)

 
 ACK
 

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


Re: [libvirt] [PATCH 6/7] qemu: remove failed create prior to root squash share path

2015-01-29 Thread Eric Blake
On 01/28/2015 03:25 PM, John Ferlan wrote:
 https://bugzilla.redhat.com/show_bug.cgi?id=1158034
 
 In qemuOpenFileAs if we fall into the path where we'll be opening / creating
 the file using VIR_FILE_OPEN_FORK, we need to first unlink/delete the file
 we created in the first path; otherwise, the attempt by the child process
 to open as some specific user:group may fail because the file was already
 created using nfsnobody:nfsnobody.
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/qemu/qemu_driver.c | 9 +
  1 file changed, 9 insertions(+)

Might be worth squashing with 5; up to you.

ACK.

 
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 89b54c8..91fefa9 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -2954,6 +2954,15 @@ qemuOpenFileAs(uid_t fallback_uid, gid_t fallback_gid,
  goto error;
  }
  
 +/* If we created the file above, then we need to remove it;
 + * otherwise, the next attempt to create will fail. If the
 + * file had already existed before we got here, then we also
 + * don't want to delete it and allow the following to succeed
 + * or fail based on existing protections
 + */
 +if (need_unlink)
 +unlink(path);
 +
  /* Retry creating the file as qemu user */
  
  if ((fd = virFileOpenAs(path, oflags,
 

-- 
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 5/7] qemu: Don't unconditionally delete file in qemuOpenFileAs

2015-01-29 Thread Eric Blake
On 01/28/2015 03:25 PM, John Ferlan wrote:
 If we're expecting to create a file somewhere and that fails for some
 reason during qemuOpenFileAs, then we unlink the path we're attempting
 to create leaving no way to determine what the existing privileges,
 protections, or labels are that caused the failure (open, change owner
 and group, change mode, etc)
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/qemu/qemu_driver.c | 3 +++
  1 file changed, 3 insertions(+)

ACK.

 
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 0174c87..89b54c8 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -2899,6 +2899,9 @@ qemuOpenFileAs(uid_t fallback_uid, gid_t fallback_gid,
  vfoflags |= VIR_FILE_OPEN_FORCE_OWNER;
  
  if (stat(path, sb) == 0) {
 +/* It already exists, we don't want to delete it on error */
 +need_unlink = false;
 +
  is_reg = !!S_ISREG(sb.st_mode);
  /* If the path is regular file which exists
   * already and dynamic_ownership is off, we don't
 

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



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

Re: [libvirt] [PATCH 1/7] qemu: Save/restore error in error path for qemuDomainSaveInternal

2015-01-29 Thread Eric Blake
On 01/28/2015 03:25 PM, John Ferlan wrote:
 If we get to endjob: because of some error earlier such as perhaps
 failure from qemuDomainSaveMemory to open/create the save file and
 the attempt to restart the CPU's fails, then we get the error from
 that restart attempt displayed to the user rather than the error
 from the failed attempt to create a save file.
 
 Upstream commit id '540c339a' changed the flow of the code moving
 the EndAsyncJob call and thus exposing the issue where an error in
 restarting CPUs resulted in the following:
 
 error: Failed to save domain rhel70 to /tmp/pl/rhel70.save
 error: internal error: unexpected async job 3
 
 where /tmp/pl is a NFS root squashed client where the failure to save
 the file (EPERM or ENOTCONN) should result in a failure:
 
 error: Failed to save domain rhel70 to /tmp/pl/rhel70.save
 error: Error from child process creating '/tmp/pl/rhel70.save': Transport 
 endpoint is not connected
 
 or (REVISIT ORDER - that is if I move the virfile check for ENOTCONN
 to sooner, then we'll never see ENOTCONN)

Is this a note about the possibility of applying the series in a
different order?

 
 error: Failed to save domain rhel70 to /tmp/pl/rhel70.save
 error: Error from child process creating '/tmp/pl/rhel70.save': Operation not 
 permitted
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/qemu/qemu_driver.c | 3 +++
  1 file changed, 3 insertions(+)
 

ACK

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



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

Re: [libvirt] [PATCH 3/7] virfile: Need to check for ENOTCONN from recvfd failure

2015-01-29 Thread Eric Blake
On 01/28/2015 03:25 PM, John Ferlan wrote:
 A gnulib change (commit id 'beae0bdc') causes ENOTCONN to be returned
 from recvfd which causes us to fall into the throwaway waitpid() call
 and return ENOTCONN to the caller, this then gets displayed during
 a 'virsh save' when using a root squashed NFS environment that's trying
 to save the file as something other than root:root.
 
 This patch will add the additional check for ENOTCONN to force the code
 into the waitpid loop looking for the actual status from the _exit()'d
 child fork.
 
 Signed-off-by: John Ferlan jfer...@redhat.com
 ---
  src/util/virfile.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

ACK.  It goes away in your later bigger patch, but doing this makes
backports of a trivial fix without the risk of a refactor bug easier.

-- 
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] Add rlimits to lxc

2015-01-29 Thread Ryan Cleere
Hello Michal,

Thank you for taking the time to review my patch, you made some very good
points and helped me discover features of the code base I didn't know.

To answer you question: systemd can still do whatever it wants within the
container this won't change that. Setting of the rlimits happens before the
init of the container is even exec'd.  So my use case for writing this
patch was that we were trying to run things that required large numbers of
open files, like apache, but when the containers started they inherited the
RLIMIT_NOFILE limits from the system init which is set conservatively low.
In a privileged container it would be no problem to increase that limit,
because you can run things as root (id=0) and you have the capability.
However we use idmap so that we don't have privileged root containers
running and as a unprivileged user you can only decrease your limit, not
increase them. This patch allows me to set the starting RLLIMIT_NOFILE
limits I want at container boot and can be changed per container.

In my next reply you will find my new patch addressing your criticisms.

Thanks
Ryan

On Wed, Dec 10, 2014 at 8:18 AM, Michal Privoznik mpriv...@redhat.com
wrote:

 On 07.12.2014 04:17, Ryan Cleere wrote:
  ---
src/conf/domain_conf.c   | 115
 +++
src/conf/domain_conf.h   |  12 +
src/lxc/lxc_controller.c |  12 +
src/util/virprocess.c|   4 +-
src/util/virprocess.h|   3 ++
5 files changed, 144 insertions(+), 2 deletions(-)

 First of all, how would this play with systemd? I mean, what if systemd
 within the container wants to set up its own limits?

 Then, you are extending the domain XML space without any documentation.
 All XML changes must go hand in hand with documentation and RNG schema
 change too.
 
  diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
  index 2d81c37..a673dc2 100644
  --- a/src/conf/domain_conf.c
  +++ b/src/conf/domain_conf.c
  @@ -26,6 +26,7 @@
#include dirent.h
#include fcntl.h
#include strings.h
  +#include sys/resource.h
#include sys/stat.h
#include unistd.h
 
  @@ -1003,7 +1004,86 @@ virDomainBlkioDeviceParseXML(xmlNodePtr root,
return -1;
}
 
  +static virDomainRLimitsPtr
  +virDomainRLimitsNew(void)
  +{
  +virDomainRLimitsPtr def = NULL;
  +
  +if (VIR_ALLOC(def)  0)
  +return NULL;
  +
  +return def;
  +}

 What's the reason for this function? I mean, unless it is setting some
 defauls, or doing something different than VIR_ALLOC() I don't think we
 need that. Esp. if there's no Free() counterpart.

  +
  +static virDomainRLimitsPtr
  +virDomainRLimitParseXML(xmlNodePtr node)
  +{
  +char *c = NULL;
  +long long val;
  +virDomainRLimitsPtr def;
 
  +if (!(def = virDomainRLimitsNew()))
  +return NULL;
  +
  +if (node-type == XML_ELEMENT_NODE) {
  +c = (char *)xmlNodeGetContent(node);
  +if (virStrToLong_ll(c, NULL, 10, val)  0) {
  +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
  +   _(could not parse rlimit value of %s),
  +   c);
  +goto error;
  +}
  +VIR_FREE(c);
  +
  +def-limit = val;
  +if (VIR_STRDUP(def-name, (char *)node-name)  0)
  +goto error;

 And here we go. If this VIR_STRDUP succeeds but the node-name is not
 known to the following huge if-else chunk, the string is leaked. Moreover,
 the def-name is just string translation of def-resource. The translation
 is static so I think you should use VIR_ENUM_{DECL,IMPL} macros rather than
 the following.

  +
  +if (xmlStrEqual(node-name, BAD_CAST as)) {
  +def-resource = RLIMIT_AS;
  +} else if (xmlStrEqual(node-name, BAD_CAST core)) {
  +def-resource = RLIMIT_CORE;
  +} else if (xmlStrEqual(node-name, BAD_CAST cpu)) {
  +def-resource = RLIMIT_CPU;
  +} else if (xmlStrEqual(node-name, BAD_CAST data)) {
  +def-resource = RLIMIT_DATA;
  +} else if (xmlStrEqual(node-name, BAD_CAST fsize)) {
  +def-resource = RLIMIT_FSIZE;
  +} else if (xmlStrEqual(node-name, BAD_CAST locks)) {
  +def-resource = RLIMIT_LOCKS;
  +} else if (xmlStrEqual(node-name, BAD_CAST memlock)) {
  +def-resource = RLIMIT_MEMLOCK;
  +} else if (xmlStrEqual(node-name, BAD_CAST msgqueue)) {
  +def-resource = RLIMIT_MSGQUEUE;
  +} else if (xmlStrEqual(node-name, BAD_CAST nice)) {
  +def-resource = RLIMIT_NICE;
  +} else if (xmlStrEqual(node-name, BAD_CAST nofile)) {
  +def-resource = RLIMIT_NOFILE;
  +} else if (xmlStrEqual(node-name, BAD_CAST nproc)) {
  +def-resource = RLIMIT_NPROC;
  +} else if (xmlStrEqual(node-name, BAD_CAST rss)) {
  +def-resource = RLIMIT_RSS;
  +} else if 

Re: [libvirt] [PATCH] cpu: add Freescale ppc64 CPU models

2015-01-29 Thread hong-hua....@freescale.com
Prerna,

Thank you. I'll update the patch according to the latest implementation.

Best Regards,
Olivia

 -Original Message-
 From: Prerna Saxena [mailto:pre...@linux.vnet.ibm.com]
 Sent: Thursday, January 29, 2015 8:01 PM
 To: Yin Olivia-R63875; libvir-list@redhat.com
 Subject: Re: [libvirt] [PATCH] cpu: add Freescale ppc64 CPU models
 
 
 On Friday 23 January 2015 08:42 AM, Olivia Yin wrote:
  Signed-off-by: Olivia Yin hong-hua@freescale.com
  ---
   src/cpu/cpu_map.xml | 38 +-
   1 file changed, 37 insertions(+), 1 deletion(-)
 
  diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml index
  bd9b056..c34874e 100644
  --- a/src/cpu/cpu_map.xml
  +++ b/src/cpu/cpu_map.xml
  @@ -1,4 +1,4 @@
  -cpus
  +ncpus
 
 This looks to be a typo.
 
 arch name='x86'
   !-- vendor definitions --
   vendor name='Intel' string='GenuineIntel'/ @@ -600,6 +600,7 @@
 arch name='ppc64'
   !-- vendor definitions --
   vendor name='IBM'/
  +vendor name='Freescale'/
 
   !-- IBM-based CPU models --
   model name='POWER7'
  @@ -657,5 +658,40 @@
 pvr value='0x004d'/
   /model
 
  +!-- Freescale-based CPU models --
  +model name='POWERPC_e5500_v10'
  +  vendor name='Freescale'/
  +  pvr value='0x80240010'/
  +/model
  +
  +model name='POWERPC_e5500_v11'
  +  vendor name='Freescale'/
  +  pvr value='0x80240011'/
  +/model
  +
  +model name='POWERPC_e5500_v12'
  +  vendor name='Freescale'/
  +  pvr value='0x80240012'/
  +/model
  +
  + model name='POWERPC_e5500_v1020'
  +  vendor name='Freescale'/
  +  pvr value='0x80241020'/
  +/model
  +
  +model name='POWERPC_e6500_v10'
  +  vendor name='Freescale'/
  +  pvr value='0x80400010'/
  +/model
  +
  +model name='POWERPC_e6500_v20'
  +  vendor name='Freescale'/
  +  pvr value='0x80400020'/
  +/model
  +
  +model name='POWERPC_e6500_v120'
  +  vendor name='Freescale'/
  +  pvr value='0x80400120'/
  +/model
 /arch
   /cpus
 Rest of the patch looks good.
 
 However, I had an observation. It appears that POWERPC_e6500_v10 , .._v20 
 _v120 seem variants of the same processor family -- they share the same higher
 16 bits of PVR.
 
 Do you need to specifically expose the different variants of these models in 
 the
 management stack ?
 Or the purpose of adding these entries is merely to enable libvirt to run on 
 these
 boards ?
 
 If the latter describes your need, you just need to add a generic entry for 
 this
 family, such as :
 
 +model name='POWERPC_e6500'
 +  vendor name='Freescale'/
 +  pvr value='0x8040'/
 +/model
 
 Libvirt driver for ppc64 currently has support to fallback to a generic entry 
 for a
 given model if the exact PVR isnt found.
 
 Regards,
 
 --
 Prerna Saxena
 
 Linux Technology Centre,
 IBM Systems and Technology Lab,
 Bangalore, India


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


Re: [libvirt] [libvirt-test-API][PATCH V2 04/11] Add IPv6 section into related network case

2015-01-29 Thread hongming


On 01/28/2015 03:14 PM, jiahu wrote:

Added 4 IPv6 required parameters to case.
---
  repos/network/define.py | 7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/repos/network/define.py b/repos/network/define.py
index dd054f7..6e50eb7 100644
--- a/repos/network/define.py
+++ b/repos/network/define.py
@@ -17,7 +17,12 @@ required_params = ('networkname',
 'bridgenetmask',
 'netstart',
 'netend',
-   'netmode',)
+   'netmode',
+   'netip6addr',
+   'netip6prefix',
+   'netip6start',
+   'netip6end',
+   )


These ipv6 params should be optional .
ACK and Pushed after modifying the patch as follows and getting the 
correct testing result


---
 repos/network/create.py | 4 
 1 file changed, 4 insertions(+)

diff --git a/repos/network/create.py b/repos/network/create.py
index 399328c..b1b3245 100644
--- a/repos/network/create.py
+++ b/repos/network/create.py
@@ -19,6 +19,10 @@ required_params = ('networkname',
'netend',
'netmode',)
 optional_params = {'xml' : 'xmls/network.xml',
+   'netip6addr' : '2001b8:ca2:98::1',
+   'netip6prefix' : '64',
+   'netip6start' : '2001b8:ca2:98::11',
+   'netip6end' : '2001b8:ca2:98::ff',
   }

 def check_network_status(*args):
--
1.8.3.1

  optional_params = {'xml' : 'xmls/network.xml',
}
  


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

[libvirt] [PATCH v2] cpu: add Freescale ppc64 CPU models

2015-01-29 Thread Olivia Yin
From: Olivia Yin hong-hua@freescale.com

With this patch, Freescale ppc64 CPU modesl could be recognized.

virsh # cpu-models ppc64
POWERPC_e6500
POWERPC_e5500
power8
power8e
power7+
power7
power6
POWER8_v1.0
POWER7+_v2.1
POWER7_v2.3
POWER7_v2.1
POWER7

virsh # capabilities
capabilities
  host
uuid5a54efa6-20dc-4da7-b271-0b7fb7062cb8/uuid
cpu
  archppc64/arch
  modelPOWERPC_e6500/model
  vendorFreescale/vendor


---
 src/cpu/cpu_map.xml | 12 
 1 file changed, 12 insertions(+)

diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index bd9b056..413148f 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -600,6 +600,7 @@
   arch name='ppc64'
 !-- vendor definitions --
 vendor name='IBM'/
+vendor name='Freescale'/
 
 !-- IBM-based CPU models --
 model name='POWER7'
@@ -657,5 +658,16 @@
   pvr value='0x004d'/
 /model
 
+!-- Freescale-based CPU models --
+model name='POWERPC_e5500'
+  vendor name='Freescale'/
+  pvr value='0x8024'/
+/model
+
+model name='POWERPC_e6500'
+  vendor name='Freescale'/
+  pvr value='0x8040'/
+/model
+
   /arch
 /cpus
-- 
2.1.0.27.g96db324

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


Re: [libvirt] [PATCH 0/2] schemas: Remove useless definition

2015-01-29 Thread Martin Kletzander

On Fri, Jan 30, 2015 at 06:55:05AM +0100, Martin Kletzander wrote:

Let's save 21 lines just because I've found out a way how to convince
xmllint to do something we thought it can't.  The first patch is the
brains of this operation, the second one is just a cleaner guy; best
served with '-w'.



Oh, never mind, NACK this.  The problem is one story deeper.  For
those interested:

With these patches, one type of XML still fails to validate; those
where e.g. vlan/ comes before virtualport/.  That is most likely
caused by virtualPortProfile being enclosed in interleave/ with
source/.  If I were to remove that, we'd be validating perfectly,
*but* it would be invalid to have source/ after virtualport/.

In other words, if you have a group and elements in that group can
interleave, the group can interleave with other options only as a
group (no element it can interleave with can be placed inside that
group) -- that makes sense, until...
If, however, elements in that group cannot interleave, it means that
those elements must be specified in the same exact order, *but*
elements the whole group can interleave with *can* be specified inside
that group without any problems.

I should've just listened to Michal and leave it like that.  xmllint-- rng--


Martin Kletzander (2):
 schema: Remove unnecessary interface-options
 schema: Indent interface properly

docs/schemas/domaincommon.rng | 787 --
1 file changed, 383 insertions(+), 404 deletions(-)

--
2.2.2

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


pgpD9B2tGM1A4.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] schemas: Allow all generic elements and attributes for all interfaces

2015-01-29 Thread Martin Kletzander

On Thu, Jan 29, 2015 at 04:12:15PM +0100, Michal Privoznik wrote:

On 29.01.2015 11:25, Martin Kletzander wrote:

On Wed, Jan 28, 2015 at 06:23:08PM +0100, Michal Privoznik wrote:

There are some interface types (notably 'server' and 'client')
which instead of allowing the default set of elements and
attributes (like the rest do), try to enumerate only the elements
they know of. This way it's, however, easy to miss something. For
instance, the address/ element was not mentioned at all. This
resulted in a strange behavior: when such interface was added
into XML, the address was automatically generated by parsing
code. Later, the formatted XML hasn't passed the RNG schema. This
became more visible once we've turned on the XML validation on
domain XML changes: appending an empty line at the end of
formatted XML (to trick virsh think the XML had changed) made
libvirt to refuse the very same XML it formatted.

Instead of trying to find each element and attribute we are
missing in the schema, lets just allow all the elements and
attributes like we're doing that for the rest of types. It's no
harm if the schema is wider than our parser allows.



I'm pretty sure that separating it was the original intention, but
until it's separated properly without breaking things, I agree with
this fix.


Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
docs/schemas/domaincommon.rng  |  28 +---
.../qemuxml2argv-interface-server.xml  | 157
+


So this file is added only for domainschematest?  Why not check it by
xml2xml as well?


Good point.



This effectively allows interface-options to be used with any
interface, why not move it after the choice/ element, so we're not
redundant?


Not a good point. I mean, I don't think one is allowed to:
element
 interlave
   choice
 group/
 group/
   choice
 /interleave
/element

simply because the choice groups may be defined by the value of an
attribute to the top parent element. Well, at least if I did it that way
I got an error from xmllint (and don't even get me started on it's error
reporting capabilities. For a long while I thought we have the worst
error reporting ever. Then I met xmllint).



Um... Yes!  And I've found out why.  It's simply because of that
reference to the 'interface-options' definition.  And you can't get
that info from xmllint.  Anyway, it is possible, albeit not so
beautiful patch-wise.  I'll Cc you on the patches that fix that.



ACK if you fix those two things.


So I'm fixing the first issue and pushing. Thanks.

Michal


pgpgY2W7bxxfL.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/2] schema: Remove unnecessary interface-options

2015-01-29 Thread Martin Kletzander
Recent commit 5568 added 'interface-options' to all interface types
rendering the definition useless.  Moreover the usage was redundant and
this patch aims to fix that.

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 docs/schemas/domaincommon.rng | 37 -
 1 file changed, 8 insertions(+), 29 deletions(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index d467dce..4ed9845 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2029,6 +2029,7 @@
 --
   define name=interface
 element name=interface
+interleave
   choice
 group
   attribute name=type
@@ -2046,7 +2047,6 @@
 optional
   ref name=virtualPortProfile/
 /optional
-ref name=interface-options/
   /interleave
 /group
 group
@@ -2062,7 +2062,6 @@
 empty/
   /element
 /optional
-ref name=interface-options/
   /interleave
 /group
 group
@@ -2087,7 +2086,6 @@
 /attribute
 empty/
   /element
-ref name=interface-options/
   /interleave
 /group
 group
@@ -2109,7 +2107,6 @@
 optional
   ref name=virtualPortProfile/
 /optional
-ref name=interface-options/
   /interleave
 /group
 group
@@ -2131,16 +2128,12 @@
 optional
   ref name=virtualPortProfile/
 /optional
-ref name=interface-options/
   /interleave
 /group
 group
   attribute name=type
 valueuser/value
   /attribute
-  interleave
-ref name=interface-options/
-  /interleave
 /group
 group
   attribute name=type
@@ -2153,7 +2146,6 @@
   /attribute
   empty/
 /element
-ref name=interface-options/
   /interleave
 /group
 group
@@ -2173,7 +2165,6 @@
   /attribute
   empty/
 /element
-ref name=interface-options/
   /interleave
 /group
 group
@@ -2192,7 +2183,6 @@
   /attribute
   empty/
 /element
-ref name=interface-options/
   /interleave
 /group
 group
@@ -2244,28 +2234,10 @@
 optional
   ref name=virtualPortProfile/
 /optional
-ref name=interface-options/
   /interleave
 /group
   /choice
   optional
-attribute name=trustGuestRxFilters
-  ref name=virYesNo/
-/attribute
-  /optional
-/element
-  /define
-  !--
-  The interface options possible are:
-- the MAC address
-- the IP address bound to the interface
-- the name of the script used to set up the binding
-- the target device used
-- boot order
---
-  define name=interface-options
-interleave
-  optional
 element name=target
   attribute name=dev
 ref name=deviceName/
@@ -2492,8 +2464,15 @@
   optional
 ref name=vlan/
   /optional
+  optional
+attribute name=trustGuestRxFilters
+  ref name=virYesNo/
+/attribute
+  /optional
 /interleave
+/element
   /define
+
   !--
   An emulator description is just a path to the binary used for the task
 --
-- 
2.2.2

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


[libvirt] [PATCH 2/2] schema: Indent interface properly

2015-01-29 Thread Martin Kletzander
After recent change, the indentation for interface definition in the
Relax-NG schema is off.  This whitespace-only patch fixes that in order
for the previous patch to be cleaner and more readable.

To be viewed with '-w' (ignoring whitespaces).

Signed-off-by: Martin Kletzander mklet...@redhat.com
---
 docs/schemas/domaincommon.rng | 762 +-
 1 file changed, 381 insertions(+), 381 deletions(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4ed9845..b8c3e0e 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2029,46 +2029,46 @@
 --
   define name=interface
 element name=interface
-interleave
-  choice
-group
-  attribute name=type
-valuebridge/value
-  /attribute
-  interleave
-optional
-  element name=source
-attribute name=bridge
-  ref name=deviceName/
-/attribute
-empty/
-  /element
-/optional
-optional
-  ref name=virtualPortProfile/
-/optional
-  /interleave
-/group
-group
-  attribute name=type
-valueethernet/value
-  /attribute
-  interleave
-optional
-  element name=source
-attribute name=dev
-  ref name=deviceName/
-/attribute
-empty/
-  /element
-/optional
-  /interleave
-/group
-group
-  attribute name=type
-valuevhostuser/value
-  /attribute
-  interleave
+  interleave
+choice
+  group
+attribute name=type
+  valuebridge/value
+/attribute
+interleave
+  optional
+element name=source
+  attribute name=bridge
+ref name=deviceName/
+  /attribute
+  empty/
+/element
+  /optional
+  optional
+ref name=virtualPortProfile/
+  /optional
+/interleave
+  /group
+  group
+attribute name=type
+  valueethernet/value
+/attribute
+interleave
+  optional
+element name=source
+  attribute name=dev
+ref name=deviceName/
+  /attribute
+  empty/
+/element
+  /optional
+/interleave
+  /group
+  group
+attribute name=type
+  valuevhostuser/value
+/attribute
+interleave
   element name=source
 attribute name=type
   choice
@@ -2086,390 +2086,390 @@
 /attribute
 empty/
   /element
-  /interleave
-/group
-group
-  attribute name=type
-valuenetwork/value
-  /attribute
-  interleave
-element name=source
-  attribute name=network
-ref name=deviceName/
-  /attribute
+/interleave
+  /group
+  group
+attribute name=type
+  valuenetwork/value
+/attribute
+interleave
+  element name=source
+attribute name=network
+  ref name=deviceName/
+/attribute
+optional
+  attribute name=portgroup
+ref name=deviceName/
+  /attribute
+/optional
+empty/
+  /element
   optional
-attribute name=portgroup
+ref name=virtualPortProfile/
+  /optional
+/interleave
+  /group
+  group
+attribute name=type
+  valuedirect/value
+/attribute
+interleave
+  element name=source
+attribute name=dev
   ref name=deviceName/
 /attribute
+optional
+  attribute name=mode
+ref name=bridgeMode/
+  /attribute
+/optional
+empty/
+  /element
+  optional
+ref name=virtualPortProfile/
   /optional
-  empty/
-/element
+/interleave
+  /group
+  group
+attribute name=type
+  valueuser/value
+/attribute
+  /group
+  group
+attribute name=type
+  valueinternal/value
+/attribute
+interleave
+  element 

[libvirt] [PATCH 0/2] schemas: Remove useless definition

2015-01-29 Thread Martin Kletzander
Let's save 21 lines just because I've found out a way how to convince
xmllint to do something we thought it can't.  The first patch is the
brains of this operation, the second one is just a cleaner guy; best
served with '-w'.

Martin Kletzander (2):
  schema: Remove unnecessary interface-options
  schema: Indent interface properly

 docs/schemas/domaincommon.rng | 787 --
 1 file changed, 383 insertions(+), 404 deletions(-)

--
2.2.2

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


[libvirt] [PATCH] Add ability to set rlimits at container boot

2015-01-29 Thread Ryan Cleere
---
 docs/formatdomain.html.in | 49 +++
 docs/schemas/domaincommon.rng | 89
+
 src/conf/domain_conf.c| 92
+++
 src/conf/domain_conf.h| 33 
 src/libvirt_private.syms  |  1 +
 src/lxc/lxc_controller.c  | 32 +++
 src/util/virprocess.c |  4 +-
 src/util/virprocess.h |  2 +
 8 files changed, 300 insertions(+), 2 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index f8d5f89..5aec51c 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -348,6 +348,55 @@
 /pre


+p
+  If you want to set an rlimit of the containter init process instead
of
+  inheriting from the host init, set the coderlimits/code element.
You
+  are able to set any of the rlimits that setrlimits is able to set
using
+  any of the following sub-elements:
+/p
+
+dl
+  dtcodeas/code/dt
+  ddUsed to set RLIMIT_AS./dd
+  dtcodecore/code/dt
+  ddUsed to set RLIMIT_CORE./dd
+  dtcodecpu/code/dt
+  ddUsed to set RLIMIT_CPU./dd
+  dtcodedata/code/dt
+  ddUsed to set RLIMIT_DATA./dd
+  dtcodefsize/code/dt
+  ddUsed to set RLIMIT_FSIZE./dd
+  dtcodelocks/code/dt
+  ddUsed to set RLIMIT_LOCKS./dd
+  dtcodememlock/code/dt
+  ddUsed to set RLIMIT_MEMLOCK./dd
+  dtcodemsgqueue/code/dt
+  ddUsed to set RLIMIT_MSGQUEUE./dd
+  dtcodenice/code/dt
+  ddUsed to set RLIMIT_NICE./dd
+  dtcodenofile/code/dt
+  ddUsed to set RLIMIT_NOFILE./dd
+  dtcodenproc/code/dt
+  ddUsed to set RLIMIT_NPROC./dd
+  dtcoderss/code/dt
+  ddUsed to set RLIMIT_RSS./dd
+  dtcodertprio/code/dt
+  ddUsed to set RLIMIT_RTPRIO./dd
+  dtcoderttime/code/dt
+  ddUsed to set RLIMIT_RTTIME./dd
+  dtcodesigpending/code/dt
+  ddUsed to set RLIMIT_SIGPENDING./dd
+  dtcodestack/code/dt
+  ddUsed to set RLIMIT_STACK./dd
+/dl
+
+pre
+  lt;rlimitsgt;
+lt;nofilegt;10240lt;/nofilegt;
+  lt;/rlimitsgt;
+/pre
+
+
 h3a name=elementsSysinfoSMBIOS System Information/a/h3

 p
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index d467dce..b98f8d5 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -59,6 +59,9 @@
   ref name=idmap/
 /optional
 optional
+  ref name=rlimits/
+/optional
+optional
   ref name=devices/
 /optional
 zeroOrMore
@@ -570,6 +573,92 @@
   /interleave
 /element
   /define
+  define name=rlimits
+element name=rlimits
+  interleave
+optional
+  element name=cpu
+ref name='scaledInteger'/
+  /element
+/optional
+optional
+  element name=fsize
+ref name='scaledInteger'/
+  /element
+/optional
+optional
+  element name=data
+ref name='scaledInteger'/
+  /element
+/optional
+optional
+  element name=stack
+ref name='scaledInteger'/
+  /element
+/optional
+optional
+  element name=core
+ref name='scaledInteger'/
+  /element
+/optional
+optional
+  element name=rss
+ref name='scaledInteger'/
+  /element
+/optional
+optional
+  element name=nproc
+ref name='scaledInteger'/
+  /element
+/optional
+optional
+  element name=nofile
+ref name='scaledInteger'/
+  /element
+/optional
+optional
+  element name=memlock
+ref name='scaledInteger'/
+  /element
+/optional
+optional
+  element name=as
+ref name='scaledInteger'/
+  /element
+/optional
+optional
+  element name=locks
+ref name='scaledInteger'/
+  /element
+/optional
+optional
+  element name=sigpending
+ref name='scaledInteger'/
+  /element
+/optional
+optional
+  element name=msgqueue
+ref name='scaledInteger'/
+  /element
+/optional
+optional
+  element name=nice
+ref name='scaledInteger'/
+  /element
+/optional
+optional
+  element name=rtprio
+ref name='scaledInteger'/
+  /element
+/optional
+optional
+  element name=rttime
+ref name='scaledInteger'/
+  /element
+/optional
+  /interleave
+/element
+  /define
   !--
   Resources usage defines the amount of memory (maximum and possibly
   current usage) and number of virtual CPUs used by that domain.
diff --git a/src/conf/domain_conf.c