Re: [libvirt] [PATCH v2] qemu: Adding 'downscript' feature for QEMU network interfaces.

2017-05-15 Thread Julio Faracco
Ping. Any feedback?

2017-05-05 11:25 GMT-03:00 Julio Faracco :
> V1 patch did not have the docs/formatdomain.html.in commit.
>
> 2017-05-05 11:22 GMT-03:00 Julio Faracco :
>> This commit adds the support for 'downscript' feature:
>> - For QEMU command line with the option:
>>   '-net downscript=/etc/qemu-ifdown,...'.
>>
>> - For Domains with a network interface description:
>>   '
>>  ...
>>  
>>  ...
>>   '
>>
>> The options 'script' and 'downscript' accept the argument 'no' to disable
>> the script executions. The way that the code was implemented, the XML file
>> accepts '<[down]script path='no'>' to solve this problem.
>>
>> This commit updates the tests and documentation too.
>>
>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=825939
>>
>> Signed-off-by: Julio Faracco 
>> ---
>>  docs/formatdomain.html.in  |  1 +
>>  docs/schemas/domaincommon.rng  |  8 
>>  src/conf/domain_conf.c | 13 
>> +
>>  src/conf/domain_conf.h |  1 +
>>  src/qemu/qemu_parse_command.c  |  4 
>>  tests/qemuargv2xmldata/qemuargv2xml-net-eth-ifname.args|  2 +-
>>  tests/qemuargv2xmldata/qemuargv2xml-net-eth-ifname.xml |  1 +
>>  tests/qemuargv2xmldata/qemuargv2xml-net-eth.args   |  2 +-
>>  tests/qemuargv2xmldata/qemuargv2xml-net-eth.xml|  1 +
>>  tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.xml |  1 +
>>  tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml|  1 +
>>  tests/qemuxml2xmloutdata/qemuxml2xmlout-net-eth-ifname.xml |  1 +
>>  tests/qemuxml2xmloutdata/qemuxml2xmlout-net-eth.xml|  1 +
>>  13 files changed, 35 insertions(+), 2 deletions(-)
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index 8c884f4..89fe86d 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -4663,6 +4663,7 @@
>>interface type='ethernet'
>>  target dev='vnet7'/
>>  script path='/etc/qemu-ifup-mynet'/
>> +downscript path='/etc/qemu-ifdown-mynet'/
>>/interface
>>  /devices
>>  ...
>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>> index 281309e..2f88dda 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -2609,6 +2609,14 @@
>>  
>>
>>
>> +
>> +  
>> +
>> +  
>> +  
>> +
>> +  
>> +  
>>  
>>
>>  
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 0ff216e..32d5720 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -1935,6 +1935,7 @@ virDomainNetDefClear(virDomainNetDefPtr def)
>>  VIR_FREE(def->backend.vhost);
>>  VIR_FREE(def->virtPortProfile);
>>  VIR_FREE(def->script);
>> +VIR_FREE(def->downscript);
>>  VIR_FREE(def->domain_name);
>>  VIR_FREE(def->ifname);
>>  VIR_FREE(def->ifname_guest);
>> @@ -9589,6 +9590,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>>  char *ifname_guest = NULL;
>>  char *ifname_guest_actual = NULL;
>>  char *script = NULL;
>> +char *downscript = NULL;
>>  char *address = NULL;
>>  char *port = NULL;
>>  char *localaddr = NULL;
>> @@ -9761,6 +9763,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>>  } else if (!script &&
>> xmlStrEqual(cur->name, BAD_CAST "script")) {
>>  script = virXMLPropString(cur, "path");
>> +} else if (!downscript &&
>> +   xmlStrEqual(cur->name, BAD_CAST "downscript")) {
>> +downscript = virXMLPropString(cur, "path");
>>  } else if (!domain_name &&
>> xmlStrEqual(cur->name, BAD_CAST "backenddomain")) {
>>  domain_name = virXMLPropString(cur, "name");
>> @@ -10074,6 +10079,10 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr 
>> xmlopt,
>>  def->script = script;
>>  script = NULL;
>>  }
>> +if (downscript != NULL) {
>> +def->downscript = downscript;
>> +downscript = NULL;
>> +}
>>  if (domain_name != NULL) {
>>  def->domain_name = domain_name;
>>  domain_name = NULL;
>> @@ -10356,6 +10365,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>>  VIR_FREE(dev);
>>  virDomainActualNetDefFree(actual);
>>  VIR_FREE(script);
>> +VIR_FREE(downscript);
>>  VIR_FREE(bridge);
>>  VIR_FREE(model);
>>  VIR_FREE(backend);
>> @@ -22158,6 +22168,9 @@ virDomainNetDefFormat(virBufferPtr buf,
>>
>>  virBufferEscapeString(buf, "\n",
>>def->script);
>> +if (def->downscript)
>> +virBufferEscapeString(buf, "\n",
>> + 

[libvirt] [PATCH] qemu: allow to control host side link status of network device

2017-05-15 Thread Vasiliy Tolstov
Signed-off-by: Vasiliy Tolstov 
---
 docs/formatdomain.html.in | 21 +
 docs/schemas/domaincommon.rng | 11 +++
 src/conf/domain_conf.c| 28 
 src/conf/domain_conf.h|  1 +
 src/qemu/qemu_hotplug.c   | 17 +
 src/qemu/qemu_interface.c |  8 
 6 files changed, 82 insertions(+), 4 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 8c884f4af9cb..dd8e6a4afa99 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -5421,6 +5421,27 @@ qemu-kvm -net nic,model=? /dev/null
   Since 0.9.5
 
 
+Modifying phisical link state
+
+...
+devices
+  interface type='ethernet'
+source
+link state='down'/
+target dev='vnet0'/
+  /interface
+/devices
+...
+
+
+  This element provides means of setting state of the phisical network 
interface.
+  Possible values for attribute state are up and
+  down. If down is specified as the value, the 
interface
+  put in down state. Default behavior if this element is unspecified is to 
have the
+  link state up.
+  Since 3.3.2
+
+
 MTU configuration
 
 ...
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 281309ec09da..89213d63b6e9 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2798,6 +2798,17 @@
   All ip-related info for either the host or guest side of an interface
   -->
   
+
+  
+
+  
+up
+down
+  
+
+
+  
+
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0ff216e3a373..b7398276af57 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9606,6 +9606,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
 char *devaddr = NULL;
 char *mode = NULL;
 char *linkstate = NULL;
+char *hostlinkstate = NULL;
 char *addrtype = NULL;
 char *domain_name = NULL;
 char *vhostuser_mode = NULL;
@@ -9654,6 +9655,10 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
 if (virDomainNetIPInfoParseXML(_("interface host IP"),
ctxt, >hostIP) < 0)
 goto error;
+
+if (!hostlinkstate)
+   hostlinkstate = virXPathString("string(./link/@state)", 
ctxt);
+
 ctxt->node = tmpnode;
 }
 if (!macaddr && xmlStrEqual(cur->name, BAD_CAST "mac")) {
@@ -10303,6 +10308,16 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
 }
 }
 
+def->hostlinkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DEFAULT;
+if (hostlinkstate != NULL) {
+if ((def->hostlinkstate = 
virDomainNetInterfaceLinkStateTypeFromString(hostlinkstate)) <= 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unknown interface link state '%s'"),
+   hostlinkstate);
+goto error;
+}
+}
+
 if (filter != NULL) {
 switch (def->type) {
 case VIR_DOMAIN_NET_TYPE_ETHERNET:
@@ -10371,6 +10386,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
 VIR_FREE(devaddr);
 VIR_FREE(mode);
 VIR_FREE(linkstate);
+VIR_FREE(hostlinkstate);
 VIR_FREE(addrtype);
 VIR_FREE(domain_name);
 VIR_FREE(trustGuestRxFilters);
@@ -22113,6 +22129,18 @@ virDomainNetDefFormat(virBufferPtr buf,
 break;
 }
 
+if (def->hostlinkstate) {
+if (sourceLines == 0) {
+virBufferAddLit(buf, "\n");
+sourceLines += 2;
+}
+virBufferAdjustIndent(buf, 2);
+virBufferAsprintf(buf, "\n",
+
virDomainNetInterfaceLinkStateTypeToString(def->hostlinkstate));
+virBufferAdjustIndent(buf, -2);
+sourceLines += 2;
+}
+
 /* if sourceLines == 0 - no  info at all so far
  *sourceLines == 1 - first line written, no terminating ">"
  *sourceLines > 1 - multiple lines, including subelements
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 09fb7aada4b2..71e12a30c2c1 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1037,6 +1037,7 @@ struct _virDomainNetDef {
 virNetDevVlan vlan;
 int trustGuestRxFilters; /* enum virTristateBool */
 int linkstate;
+int hostlinkstate;
 unsigned int mtu;
 virNetDevCoalescePtr coalesce;
 };
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index e8d29186eb32..7fc41b28d9f8 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2978,6 +2978,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
 bool needBridgeChange = false;
 bool needFilterChange = false;
 bool needLinkStateChange = false;
+bool needHostLinkStateChange 

[libvirt] [PATCH] allow to control host side link status of ethernet network device

2017-05-15 Thread Vasiliy Tolstov
Back to old thread with Laine Stump with message title:
"qemu: remove unnecessary setting of tap device online state"
I'm not tested ip and route assign in case of up/down link and
device update on the fly. But host side link status tested and worked
fine.

Vasiliy Tolstov (1):
  qemu: allow to control host side link status of network device

 docs/formatdomain.html.in | 21 +
 docs/schemas/domaincommon.rng | 11 +++
 src/conf/domain_conf.c| 28 
 src/conf/domain_conf.h|  1 +
 src/qemu/qemu_hotplug.c   | 17 +
 src/qemu/qemu_interface.c |  8 
 6 files changed, 82 insertions(+), 4 deletions(-)

-- 
2.9.3

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


[libvirt] limit pps via bandwidth

2017-05-15 Thread Vasiliy Tolstov
Hi. I have a problem that some vps generate very big pps (50). I
want to limit it for some reasonable value. iptables does not support
by default limit by pps more that 1, i can use nft... but
Why not use tc for this ?
http://www.lartc.org/manpages/tc-pbfifo.html
http://man7.org/linux/man-pages/man8/tc-sfb.8.html

As i understand firstly we can limit by pps and secondly via bandwidth...
What do you think?

-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru

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


Re: [libvirt] [PATCH 09/10] appmor, virt-aa-helper: Add 9p support

2017-05-15 Thread Guido Günther
On Mon, May 15, 2017 at 03:23:18PM +0200, Stefan Bader wrote:
> From: Serge Hallyn 
> 
> Add fowner and fsetid to libvirt-qemu profile and add link
> to 9p file options in virt-aa-helper.
> 
> Bug-Ubuntu: https://bugs.launchpad.net/bugs/1378434
> 
> Signed-off-by: Christian Ehrhardt 
> Signed-off-by: Stefan Bader 
> ---
>  examples/apparmor/libvirt-qemu | 4 
>  src/security/virt-aa-helper.c  | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
> index 89466c9..f04ce04 100644
> --- a/examples/apparmor/libvirt-qemu
> +++ b/examples/apparmor/libvirt-qemu
> @@ -13,6 +13,10 @@
>capability setgid,
>capability setuid,
>  
> +  # for 9p
> +  capability fsetid,
> +  capability fowner,
> +
>network inet stream,
>network inet6 stream,

I would put this into a separate patch.

>  
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> index a2d5c21..667241b 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -1108,7 +1108,7 @@ get_files(vahControl * ctl)
>  /* We don't need to add deny rw rules for readonly mounts,
>   * this can only lead to troubles when mounting / readonly.
>   */
> -if (vah_add_path(, fs->src->path, fs->readonly ? "R" : "rw", 
> true) != 0)
> +if (vah_add_path(, fs->src->path, fs->readonly ? "R" : 
> "rwl", true) != 0)

Given the recent QEMU 9pfs CVS that allowed to access paths outside src.path
I would feel better if the rule produces s.th. like

 link subset src.path/** -> src.path/**,

instead of allowing links to /**.
Cheers,
 -- Guido


>  goto cleanup;
>  }
>  }
> -- 
> 2.7.4
> 

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


Re: [libvirt] [PATCH 01/10] virt-aa-helper: Ask for no deny rule for readonly disk elements

2017-05-15 Thread Stefan Bader
On 15.05.2017 17:48, Guido Günther wrote:
> On Mon, May 15, 2017 at 03:23:10PM +0200, Stefan Bader wrote:
>> From: Serge Hallyn 
>>
>> Just because a disk element only requests read access doesn't mean
>> there may not be another readwrite request.
>>
>> Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/bugs/1554031
> 
> The URL is wrong (drop the "ubuntu" part. From the bug report this looks
> like a workaround:

Darn, thanks for catching this.

> 
>  https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1554031/comments/8
> 
> or am I misreading this? Shouldn't we only change to 'w' on blockcommit?

As I understand it, the 'r' would implicitly add a write deny rule, so it is not
possible to override that. With the 'R' notation only a rule allowing read is
added. Which allows to change to change to 'w' on blockcommit.

-Stefan

> Cheers
>  -- Guido
> 
>>
>> Signed-off-by: Christian Ehrhardt 
>> Signed-off-by: Stefan Bader 
>> ---
>>  src/security/virt-aa-helper.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
>> index 5f5d1cd..d976a00 100644
>> --- a/src/security/virt-aa-helper.c
>> +++ b/src/security/virt-aa-helper.c
>> @@ -888,11 +888,11 @@ add_file_path(virDomainDiskDefPtr disk,
>>  
>>  if (depth == 0) {
>>  if (disk->src->readonly)
>> -ret = vah_add_file(buf, path, "r");
>> +ret = vah_add_file(buf, path, "R");
>>  else
>>  ret = vah_add_file(buf, path, "rw");
>>  } else {
>> -ret = vah_add_file(buf, path, "r");
>> +ret = vah_add_file(buf, path, "R");
>>  }
>>  
>>  if (ret != 0)
>> -- 
>> 2.7.4
>>
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 




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 06/10] apparmor, virt-aa-helper: Additional explicit denies for host devices

2017-05-15 Thread Stefan Bader
On 15.05.2017 17:56, Guido Günther wrote:
> On Mon, May 15, 2017 at 03:23:15PM +0200, Stefan Bader wrote:
>> From: Christian Ehrhardt 
>>
>> This adds further explicit denies for host devices to silence
>> (acceptable) denial warnings.
>>
>> Signed-off-by: Christian Ehrhardt 
>> Signed-off-by: Stefan Bader 
>> ---
>>  examples/apparmor/usr.lib.libvirt.virt-aa-helper | 4 
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/examples/apparmor/usr.lib.libvirt.virt-aa-helper 
>> b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
>> index 7804b72..012080c 100644
>> --- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper
>> +++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
>> @@ -24,6 +24,10 @@ profile virt-aa-helper 
>> /usr/{lib,lib64}/libvirt/virt-aa-helper {
>>deny /dev/sd* r,
>>deny /dev/vd* r,
>>deny /dev/dm-* r,
>> +  deny /dev/drbd[0-9]* r,
>> +  deny /dev/dasd* r,
>> +  deny /dev/nvme* r,
>> +  deny /dev/zd[0-9]* r,
>>deny /dev/mapper/ r,
>>deny /dev/mapper/* r,
> 
> This could IMHO be squashed into the previous patch since it just
> extends the list.

I agree. another case of being not certain how to proceed with changes from
different origins.

-Stefan

> Cheers,
>  -- Guido
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 




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

Re: [libvirt] [libvirt-python PATCH v2] spec: Install egg-info with rpm package

2017-05-15 Thread Daniel P. Berrange
On Mon, May 15, 2017 at 05:58:47PM +0200, Martin Kletzander wrote:
> This was being done due to now deprecated policy and that file should
> be installed so that pip can recognize that the packages is already
> installed in the system.
> 
> Signed-off-by: Martin Kletzander 
> ---
> v2:
>  - Put each egg-info ito its respective RPM package
> 
>  libvirt-python.spec.in | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

ACK


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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


[libvirt] [PATCH] remove hack for debian etch limits.h

2017-05-15 Thread Daniel P. Berrange
The debian etch distro was end-of-life a long time ago so we no
longer need the ULLONG_MAX hack. In any case gnulib now provides
an equivalent fix by default, and so our definition now triggers
syntax-check rule failure

src/internal.h:#define ULLONG_MAX   ULONG_LONG_MAX
maint.mk: define the above via some gnulib .h file
maint.mk:843: recipe for target 'sc_prohibit_always-defined_macros' failed

Signed-off-by: Daniel P. Berrange 
---

Pushed as a build break fix

 src/internal.h | 5 -
 1 file changed, 5 deletions(-)

diff --git a/src/internal.h b/src/internal.h
index 713734c..5a5a430 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -114,11 +114,6 @@
 #define __GNUC_PREREQ(maj, min) 0
 #   endif
 
-/* Work around broken limits.h on debian etch */
-#   if defined _GCC_LIMITS_H_ && ! defined ULLONG_MAX
-#define ULLONG_MAX   ULONG_LONG_MAX
-#   endif
-
 #  endif /* __GNUC__ */
 
 /**
-- 
2.9.3

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


Re: [libvirt] [PATCH 03/10] apparmor, virt-aa-helper: Allow aarch64 UEFI.

2017-05-15 Thread Guido Günther
On Mon, May 15, 2017 at 03:23:12PM +0200, Stefan Bader wrote:
> From: William Grant 
> 
> Allow access to aarch64 UEFI images.
> 
> Signed-off-by: Christian Ehrhardt 
> Signed-off-by: Stefan Bader 
> ---
>  examples/apparmor/libvirt-qemu | 2 ++
>  src/security/virt-aa-helper.c  | 4 +++-
>  tests/virt-aa-helper-test  | 2 ++
>  3 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
> index e0988bb..89466c9 100644
> --- a/examples/apparmor/libvirt-qemu
> +++ b/examples/apparmor/libvirt-qemu
> @@ -71,6 +71,8 @@
>/usr/share/seabios/** r,
>/usr/share/ovmf/** r,
>/usr/share/OVMF/** r,
> +  /usr/share/AAVMF/** r,
> +  /usr/share/qemu-efi/** r,
>  
># access PKI infrastructure
>/etc/pki/libvirt-vnc/** r,
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> index dd166c2..a2d5c21 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -513,7 +513,9 @@ valid_path(const char *path, const bool readonly)
>  "/initrd",
>  "/initrd.img",
>  "/usr/share/OVMF/",  /* for OVMF images */
> -"/usr/share/ovmf/"   /* for OVMF images */
> +"/usr/share/ovmf/",  /* for OVMF images */
> +"/usr/share/AAVMF/", /* for AAVMF images */
> +"/usr/share/qemu-efi/"   /* for AAVMF images */
>  };
>  /* override the above with these */
>  const char * const override[] = {
> diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test
> index 73f3080..51072f6 100755
> --- a/tests/virt-aa-helper-test
> +++ b/tests/virt-aa-helper-test
> @@ -307,6 +307,8 @@ testme "0" "kernel" "-r -u $valid_uuid" "$test_xml"
>  
>  testfw "ovmf (old path)" "/usr/share/ovmf/OVMF.fd"
>  testfw "OVMF (new path)" "/usr/share/OVMF/OVMF_CODE.fd"
> +testfw "AAVMF" "/usr/share/AAVMF/AAVMF_CODE.fd"
> +testfw "qemu-efi" "/usr/share/qemu-efi/QEMU_EFI.fd"
>  
>  sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e 
> "s,,$tmpdir/initrd,g" "$template_xml" > "$test_xml"
>  touch "$tmpdir/initrd"
> -- 
> 2.7.4
> 
ACK.
 -- Guido

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


Re: [libvirt] [PATCH 08/10] apparmor: provide local override templates

2017-05-15 Thread Guido Günther
On Mon, May 15, 2017 at 03:23:17PM +0200, Stefan Bader wrote:
> Local overrides is a feature Debian/Ubuntu libvirt provided for a while.
> This allows the user to have a non-conffile that he can use to extend the
> package delivered rules with extra content matching his special case.
> 
> This change provides override templates which the user can extend
> and modifies the makefile template to include those when installing
> the apparmor profiles.
> 
> Signed-off-by: Christian Ehrhardt 
> Signed-off-by: Stefan Bader 
> ---
>  examples/Makefile.am   | 14 ++
>  examples/apparmor/local-usr.lib.libvirt.virt-aa-helper |  2 ++
>  examples/apparmor/local-usr.sbin.libvirtd  |  2 ++
>  3 files changed, 18 insertions(+)
>  create mode 100644 examples/apparmor/local-usr.lib.libvirt.virt-aa-helper
>  create mode 100644 examples/apparmor/local-usr.sbin.libvirtd
> 
> diff --git a/examples/Makefile.am b/examples/Makefile.am
> index 2956e14..16c7bf6 100644
> --- a/examples/Makefile.am
> +++ b/examples/Makefile.am
> @@ -25,6 +25,8 @@ EXTRA_DIST = \
>   apparmor/libvirt-lxc \
>   apparmor/usr.lib.libvirt.virt-aa-helper \
>   apparmor/usr.sbin.libvirtd \
> + apparmor/local-usr.sbin.libvirtd \
> + apparmor/local-usr.lib.libvirt.virt-aa-helper \
>   lxcconvert/virt-lxc-convert \
>   polkit/libvirt-acl.rules \
>   $(wildcard $(srcdir)/systemtap/*.stp) \
> @@ -74,6 +76,18 @@ apparmor_DATA = \
>   apparmor/usr.sbin.libvirtd \
>   $(NULL)
>  
> +localdir = $(apparmordir)/local
> +local_DATA = \
> + apparmor/local-usr.sbin.libvirtd \
> + apparmor/local-usr.lib.libvirt.virt-aa-helper \
> + $(NULL)
> +
> +install-data-hook:
> + mv $(DESTDIR)$(localdir)/local-usr.sbin.libvirtd \
> +$(DESTDIR)$(localdir)/usr.sbin.libvirtd
> + mv $(DESTDIR)$(localdir)/local-usr.lib.libvirt.virt-aa-helper \
> +$(DESTDIR)$(localdir)/usr.lib.libvirt.virt-aa-helper
> +
>  abstractionsdir = $(apparmordir)/abstractions
>  abstractions_DATA = \
>   apparmor/libvirt-qemu \
> diff --git a/examples/apparmor/local-usr.lib.libvirt.virt-aa-helper 
> b/examples/apparmor/local-usr.lib.libvirt.virt-aa-helper
> new file mode 100644
> index 000..82c9c39
> --- /dev/null
> +++ b/examples/apparmor/local-usr.lib.libvirt.virt-aa-helper
> @@ -0,0 +1,2 @@
> +# Site-specific additions and overrides for usr.lib.libvirt.virt-aa-helper.
> +# For more details, please see /etc/apparmor.d/local/README.
> diff --git a/examples/apparmor/local-usr.sbin.libvirtd 
> b/examples/apparmor/local-usr.sbin.libvirtd
> new file mode 100644
> index 000..6e19f20
> --- /dev/null
> +++ b/examples/apparmor/local-usr.sbin.libvirtd
> @@ -0,0 +1,2 @@
> +# Site-specific additions and overrides for usr.sbin.libvirtd.
> +# For more details, please see /etc/apparmor.d/local/README.

I wonder if this is too much distro speifics? (We're shipping the same in
Debian). It should in any case be squashed into the previous commit.
Cheers,
 -- Guido

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


[libvirt] [libvirt-python PATCH v2] spec: Install egg-info with rpm package

2017-05-15 Thread Martin Kletzander
This was being done due to now deprecated policy and that file should
be installed so that pip can recognize that the packages is already
installed in the system.

Signed-off-by: Martin Kletzander 
---
v2:
 - Put each egg-info ito its respective RPM package

 libvirt-python.spec.in | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libvirt-python.spec.in b/libvirt-python.spec.in
index 5ad029281c52..fc30564133fe 100644
--- a/libvirt-python.spec.in
+++ b/libvirt-python.spec.in
@@ -65,7 +65,6 @@ CFLAGS="$RPM_OPT_FLAGS" %{__python3} setup.py build
 %if %{with_python3}
 %{__python3} setup.py install --skip-build --root=%{buildroot}
 %endif
-rm -f %{buildroot}%{_libdir}/python*/site-packages/*egg-info

 %check
 %{__python} setup.py test
@@ -80,6 +79,7 @@ rm -f %{buildroot}%{_libdir}/python*/site-packages/*egg-info
 %{_libdir}/python2*/site-packages/libvirt_qemu.py*
 %{_libdir}/python2*/site-packages/libvirt_lxc.py*
 %{_libdir}/python2*/site-packages/libvirtmod*
+%{_libdir}/python2*/site-packages/*egg-info

 %if %{with_python3}
 %files -n libvirt-python3
@@ -94,6 +94,7 @@ rm -f %{buildroot}%{_libdir}/python*/site-packages/*egg-info
 %{_libdir}/python3*/site-packages/__pycache__/libvirt_lxc.cpython-*.py*
 %{_libdir}/python3*/site-packages/__pycache__/libvirtaio.cpython-*.py*
 %{_libdir}/python3*/site-packages/libvirtmod*
+%{_libdir}/python3*/site-packages/*egg-info
 %endif

 %changelog
--
2.13.0

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


Re: [libvirt] [libvirt-python PATCH] spec: Install egg-info with rpm package

2017-05-15 Thread Martin Kletzander

On Mon, May 15, 2017 at 04:30:17PM +0100, Daniel P. Berrange wrote:

On Mon, May 15, 2017 at 05:26:39PM +0200, Martin Kletzander wrote:

This was being done due to now deprecated policy and that file should
be installed so that pip can recognize that the packages is already
installed in the system.

Signed-off-by: Martin Kletzander 
---
 libvirt-python.spec.in | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libvirt-python.spec.in b/libvirt-python.spec.in
index 5ad029281c52..132183c93c02 100644
--- a/libvirt-python.spec.in
+++ b/libvirt-python.spec.in
@@ -65,7 +65,6 @@ CFLAGS="$RPM_OPT_FLAGS" %{__python3} setup.py build
 %if %{with_python3}
 %{__python3} setup.py install --skip-build --root=%{buildroot}
 %endif
-rm -f %{buildroot}%{_libdir}/python*/site-packages/*egg-info

 %check
 %{__python} setup.py test
@@ -81,6 +80,8 @@ rm -f %{buildroot}%{_libdir}/python*/site-packages/*egg-info
 %{_libdir}/python2*/site-packages/libvirt_lxc.py*
 %{_libdir}/python2*/site-packages/libvirtmod*

+%{_libdir}/python*/site-packages/*egg-info


That wildcard looks like it'll put the python3 egg info in the python2 RPM



You are absolutely right.  I didn't realize it's two different rpm
packages.  Fix is on the way.


+
 %if %{with_python3}
 %files -n libvirt-python3
 %defattr(-,root,root)


Regards,
Daniel
--
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|


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

Re: [libvirt] [PATCH 02/10] apparmor, virt-aa-helper: allow /usr/share/OVMF/ too

2017-05-15 Thread Guido Günther
On Mon, May 15, 2017 at 03:23:11PM +0200, Stefan Bader wrote:
> From: Simon McVittie 
> 
> The split firmware and variables files introduced by
> https://bugs.debian.org/764918 are in a different directory for some reason.
> Let the virtual machine read both.
> 
> Extended by Christian Ehrhardt to generalize FW test (simplifies
> additional testing on firmware files in future).
> 
> Signed-off-by: Christian Ehrhardt 
> Signed-off-by: Stefan Bader 
> ---
>  examples/apparmor/libvirt-qemu |  1 +
>  src/security/virt-aa-helper.c  |  1 +
>  tests/virt-aa-helper-test  | 24 
>  3 files changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
> index a9020aa..e0988bb 100644
> --- a/examples/apparmor/libvirt-qemu
> +++ b/examples/apparmor/libvirt-qemu
> @@ -70,6 +70,7 @@
>/usr/share/vgabios/** r,
>/usr/share/seabios/** r,
>/usr/share/ovmf/** r,
> +  /usr/share/OVMF/** r,
>  
># access PKI infrastructure
>/etc/pki/libvirt-vnc/** r,
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> index d976a00..dd166c2 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -512,6 +512,7 @@ valid_path(const char *path, const bool readonly)
>  "/vmlinuz",
>  "/initrd",
>  "/initrd.img",
> +"/usr/share/OVMF/",  /* for OVMF images */
>  "/usr/share/ovmf/"   /* for OVMF images */
>  };
>  /* override the above with these */
> diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test
> index 68e9399..73f3080 100755
> --- a/tests/virt-aa-helper-test
> +++ b/tests/virt-aa-helper-test
> @@ -145,6 +145,20 @@ testme() {
>  fi
>  }
>  
> +testfw() {
> +title="$1"
> +fwpath="$2"
> +
> +if [ -f "$fwpath" ]; then
> +sed -e "s,###UUID###,$uuid,g"  \
> +-e "s,###DISK###,$disk1,g" \
> +-e "s,, type='pflash'>$fwpath,g" "$template_xml" > "$test_xml"
> +testme "0" "$title" "-r -u $valid_uuid" "$test_xml"
> +else
> +echo "Skipping FW $title test. Could not find $fwpath"
> +fi
> +}
> +
>  # Expected failures
>  echo "Expected failures:" >$output
>  testme "1" "invalid arg" "-z"
> @@ -291,14 +305,8 @@ sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" 
> -e "s,,$tm
>  touch "$tmpdir/kernel"
>  testme "0" "kernel" "-r -u $valid_uuid" "$test_xml"
>  
> -if [ -f /usr/share/ovmf/OVMF.fd ]; then
> -sed -e "s,###UUID###,$uuid,g"  \
> --e "s,###DISK###,$disk1,g" \
> --e "s,, type='pflash'>/usr/share/ovmf/OVMF.fd,g" "$template_xml" > 
> "$test_xml"
> -testme "0" "ovmf" "-r -u $valid_uuid" "$test_xml"
> -else
> -echo "Skipping OVMF test. Could not find /usr/share/ovmf/OVMF.fd"
> -fi
> +testfw "ovmf (old path)" "/usr/share/ovmf/OVMF.fd"
> +testfw "OVMF (new path)" "/usr/share/OVMF/OVMF_CODE.fd"
>  
>  sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e 
> "s,,$tmpdir/initrd,g" "$template_xml" > "$test_xml"
>  touch "$tmpdir/initrd"
> -- 
> 2.7.4
> 

ACK.
  -- Guido

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


Re: [libvirt] [PATCH 06/10] apparmor, virt-aa-helper: Additional explicit denies for host devices

2017-05-15 Thread Guido Günther
On Mon, May 15, 2017 at 03:23:15PM +0200, Stefan Bader wrote:
> From: Christian Ehrhardt 
> 
> This adds further explicit denies for host devices to silence
> (acceptable) denial warnings.
> 
> Signed-off-by: Christian Ehrhardt 
> Signed-off-by: Stefan Bader 
> ---
>  examples/apparmor/usr.lib.libvirt.virt-aa-helper | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/examples/apparmor/usr.lib.libvirt.virt-aa-helper 
> b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
> index 7804b72..012080c 100644
> --- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper
> +++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
> @@ -24,6 +24,10 @@ profile virt-aa-helper 
> /usr/{lib,lib64}/libvirt/virt-aa-helper {
>deny /dev/sd* r,
>deny /dev/vd* r,
>deny /dev/dm-* r,
> +  deny /dev/drbd[0-9]* r,
> +  deny /dev/dasd* r,
> +  deny /dev/nvme* r,
> +  deny /dev/zd[0-9]* r,
>deny /dev/mapper/ r,
>deny /dev/mapper/* r,

This could IMHO be squashed into the previous patch since it just
extends the list.
Cheers,
 -- Guido

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


Re: [libvirt] [PATCH 01/10] virt-aa-helper: Ask for no deny rule for readonly disk elements

2017-05-15 Thread Guido Günther
On Mon, May 15, 2017 at 03:23:10PM +0200, Stefan Bader wrote:
> From: Serge Hallyn 
> 
> Just because a disk element only requests read access doesn't mean
> there may not be another readwrite request.
> 
> Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/bugs/1554031

The URL is wrong (drop the "ubuntu" part. From the bug report this looks
like a workaround:

 https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1554031/comments/8

or am I misreading this? Shouldn't we only change to 'w' on blockcommit?
Cheers
 -- Guido

> 
> Signed-off-by: Christian Ehrhardt 
> Signed-off-by: Stefan Bader 
> ---
>  src/security/virt-aa-helper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> index 5f5d1cd..d976a00 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -888,11 +888,11 @@ add_file_path(virDomainDiskDefPtr disk,
>  
>  if (depth == 0) {
>  if (disk->src->readonly)
> -ret = vah_add_file(buf, path, "r");
> +ret = vah_add_file(buf, path, "R");
>  else
>  ret = vah_add_file(buf, path, "rw");
>  } else {
> -ret = vah_add_file(buf, path, "r");
> +ret = vah_add_file(buf, path, "R");
>  }
>  
>  if (ret != 0)
> -- 
> 2.7.4
> 

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


Re: [libvirt] [PATCH 04/10] apparmor, virt-aa-helper: Allow access to libnl-3 config files

2017-05-15 Thread Guido Günther
On Mon, May 15, 2017 at 03:23:13PM +0200, Stefan Bader wrote:
> From: Felix Geyer 
> 
> Allow access to libnl-3 config files
> 
> Signed-off-by: Christian Ehrhardt 
> Signed-off-by: Stefan Bader 
> ---
>  examples/apparmor/usr.lib.libvirt.virt-aa-helper | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/examples/apparmor/usr.lib.libvirt.virt-aa-helper 
> b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
> index 4a8f197..ee53c2c 100644
> --- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper
> +++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
> @@ -16,6 +16,8 @@ profile virt-aa-helper 
> /usr/{lib,lib64}/libvirt/virt-aa-helper {
>owner @{PROC}/[0-9]*/status r,
>@{PROC}/filesystems r,
>  
> +  /etc/libnl-3/classid r,
> +
># for hostdev
>/sys/devices/ r,
>/sys/devices/** r,
> -- 

ACK (shipped in a similar form in Debian already).
Cheers,
 -- Guido

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


Re: [libvirt] [PATCH 05/10] apparmor, virt-aa-helper: Explicit denies for host devices

2017-05-15 Thread Guido Günther
On Mon, May 15, 2017 at 03:23:14PM +0200, Stefan Bader wrote:
> From: Felix Geyer 
> 
> Add explicit denies for disk devices to avoid cluttering dmesg with
> (acceptable) denials.
> 
> Signed-off-by: Christian Ehrhardt 
> Signed-off-by: Stefan Bader 
> ---
>  examples/apparmor/usr.lib.libvirt.virt-aa-helper | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/examples/apparmor/usr.lib.libvirt.virt-aa-helper 
> b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
> index ee53c2c..7804b72 100644
> --- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper
> +++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
> @@ -21,6 +21,11 @@ profile virt-aa-helper 
> /usr/{lib,lib64}/libvirt/virt-aa-helper {
># for hostdev
>/sys/devices/ r,
>/sys/devices/** r,
> +  deny /dev/sd* r,
> +  deny /dev/vd* r,
> +  deny /dev/dm-* r,
> +  deny /dev/mapper/ r,
> +  deny /dev/mapper/* r,
>  
>/usr/{lib,lib64}/libvirt/virt-aa-helper mr,
>/{usr/,}sbin/apparmor_parser Ux,
> -- 
> 2.7.4

ACK (shipped in a similar form in Debian already).
 -- Guido

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


Re: [libvirt] [libvirt-python PATCH] spec: Install egg-info with rpm package

2017-05-15 Thread Daniel P. Berrange
On Mon, May 15, 2017 at 05:26:39PM +0200, Martin Kletzander wrote:
> This was being done due to now deprecated policy and that file should
> be installed so that pip can recognize that the packages is already
> installed in the system.
> 
> Signed-off-by: Martin Kletzander 
> ---
>  libvirt-python.spec.in | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/libvirt-python.spec.in b/libvirt-python.spec.in
> index 5ad029281c52..132183c93c02 100644
> --- a/libvirt-python.spec.in
> +++ b/libvirt-python.spec.in
> @@ -65,7 +65,6 @@ CFLAGS="$RPM_OPT_FLAGS" %{__python3} setup.py build
>  %if %{with_python3}
>  %{__python3} setup.py install --skip-build --root=%{buildroot}
>  %endif
> -rm -f %{buildroot}%{_libdir}/python*/site-packages/*egg-info
> 
>  %check
>  %{__python} setup.py test
> @@ -81,6 +80,8 @@ rm -f %{buildroot}%{_libdir}/python*/site-packages/*egg-info
>  %{_libdir}/python2*/site-packages/libvirt_lxc.py*
>  %{_libdir}/python2*/site-packages/libvirtmod*
> 
> +%{_libdir}/python*/site-packages/*egg-info

That wildcard looks like it'll put the python3 egg info in the python2 RPM

> +
>  %if %{with_python3}
>  %files -n libvirt-python3
>  %defattr(-,root,root)

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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


[libvirt] [libvirt-python PATCH] spec: Install egg-info with rpm package

2017-05-15 Thread Martin Kletzander
This was being done due to now deprecated policy and that file should
be installed so that pip can recognize that the packages is already
installed in the system.

Signed-off-by: Martin Kletzander 
---
 libvirt-python.spec.in | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libvirt-python.spec.in b/libvirt-python.spec.in
index 5ad029281c52..132183c93c02 100644
--- a/libvirt-python.spec.in
+++ b/libvirt-python.spec.in
@@ -65,7 +65,6 @@ CFLAGS="$RPM_OPT_FLAGS" %{__python3} setup.py build
 %if %{with_python3}
 %{__python3} setup.py install --skip-build --root=%{buildroot}
 %endif
-rm -f %{buildroot}%{_libdir}/python*/site-packages/*egg-info

 %check
 %{__python} setup.py test
@@ -81,6 +80,8 @@ rm -f %{buildroot}%{_libdir}/python*/site-packages/*egg-info
 %{_libdir}/python2*/site-packages/libvirt_lxc.py*
 %{_libdir}/python2*/site-packages/libvirtmod*

+%{_libdir}/python*/site-packages/*egg-info
+
 %if %{with_python3}
 %files -n libvirt-python3
 %defattr(-,root,root)
-- 
2.13.0

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


Re: [libvirt] [PATCH] maint: update to latest gnulib

2017-05-15 Thread Andrea Bolognani
On Mon, 2017-05-15 at 10:08 +0100, Daniel P. Berrange wrote:
> This pulls in the fixes for poll() on Win32 which finally
> makes the remote driver work again.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  .gnulib | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/.gnulib b/.gnulib
> index 94386a1..da830b5 16
> --- a/.gnulib
> +++ b/.gnulib
> @@ -1 +1 @@
> -Subproject commit 94386a13667c645fd42544a7fd302c39fcdf
> +Subproject commit da830b5146cb553ac2a4bcfe76caeb57bda24cc3

ACK

-- 
Andrea Bolognani / Red Hat / Virtualization

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


[libvirt] [PATCH v2 0/5] qemu: Unbreak aarch64/virt TCG guests

2017-05-15 Thread Andrea Bolognani
Changes from v1:

  * address review comments in 3/5;
  * all other patches are unchanged.


Andrea Bolognani (5):
  qemu: Use qemuDomainMachineIsVirt() more
  tests: Check default GIC version for aarch64/virt TCG guests
  qemu: Use GICv2 for aarch64/virt TCG guests
  gic: Remove VIR_GIC_VERSION_DEFAULT
  news: Update for GIC version on TCG changes

 docs/news.xml  | 11 +
 src/qemu/qemu_capabilities.c   |  7 +-
 src/qemu/qemu_command.c|  6 ++---
 src/qemu/qemu_domain.c | 26 +++---
 src/util/virgic.h  |  3 ---
 .../qemuxml2argv-aarch64-gic-none-tcg.args | 19 
 .../qemuxml2argv-aarch64-gic-none-tcg.xml  | 17 ++
 tests/qemuxml2argvtest.c   |  3 +++
 .../qemuxml2xmlout-aarch64-gic-none-tcg.xml| 25 +
 tests/qemuxml2xmltest.c|  1 +
 10 files changed, 103 insertions(+), 15 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-gic-none-tcg.xml

-- 
2.7.4

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


[libvirt] [PATCH v2 3/5] qemu: Use GICv2 for aarch64/virt TCG guests

2017-05-15 Thread Andrea Bolognani
There are currently some limitations in the emulated GICv3
that make it unsuitable as a default. Use GICv2 instead.

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

Signed-off-by: Andrea Bolognani 
---
 src/qemu/qemu_domain.c | 23 +-
 .../qemuxml2argv-aarch64-gic-none-tcg.args |  2 +-
 .../qemuxml2xmlout-aarch64-gic-none-tcg.xml|  2 +-
 3 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index cc02c80..079a134 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2563,6 +2563,24 @@ qemuDomainDefEnableDefaultFeatures(virDomainDefPtr def,
 for (version = VIR_GIC_VERSION_LAST - 1;
  version > VIR_GIC_VERSION_NONE;
  version--) {
+
+/* We want to use the highest available GIC version for guests;
+ * however, the emulated GICv3 is currently lacking a MSI 
controller,
+ * making it unsuitable for the pure PCIe topology we aim for.
+ *
+ * For that reason, we skip this step entirely for TCG guests,
+ * and rely on the code below to pick the default version, GICv2,
+ * which supports all the features we need.
+ *
+ * We'll want to revisit this once MSI support for GICv3 has been
+ * implemented in QEMU.
+ *
+ * See https://bugzilla.redhat.com/show_bug.cgi?id=1414081 */
+if (version == VIR_GIC_VERSION_3 &&
+def->virtType == VIR_DOMAIN_VIRT_QEMU) {
+continue;
+}
+
 if (virQEMUCapsSupportsGICVersion(qemuCaps,
   def->virtType,
   version)) {
@@ -2580,8 +2598,11 @@ qemuDomainDefEnableDefaultFeatures(virDomainDefPtr def,
 
 /* Use the default GIC version if no version was specified */
 if (def->features[VIR_DOMAIN_FEATURE_GIC] == VIR_TRISTATE_SWITCH_ON &&
-def->gic_version == VIR_GIC_VERSION_NONE)
+def->gic_version == VIR_GIC_VERSION_NONE) {
+VIR_DEBUG("Using GIC version %s (default)",
+  virGICVersionTypeToString(VIR_GIC_VERSION_DEFAULT));
 def->gic_version = VIR_GIC_VERSION_DEFAULT;
+}
 }
 
 
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.args 
b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.args
index 975a014..52b6996 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.args
@@ -7,7 +7,7 @@ QEMU_AUDIO_DRV=none \
 /usr/bin/qemu-system-aarch64 \
 -name guest \
 -S \
--machine virt,accel=tcg,gic-version=3 \
+-machine virt,accel=tcg \
 -cpu cortex-a57 \
 -m 1024 \
 -smp 1,sockets=1,cores=1,threads=1 \
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-gic-none-tcg.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-gic-none-tcg.xml
index 69510e2..a0cd0b7 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-gic-none-tcg.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-gic-none-tcg.xml
@@ -9,7 +9,7 @@
 
   
   
-
+
   
   
 cortex-a57
-- 
2.7.4

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


[libvirt] [PATCH v2 4/5] gic: Remove VIR_GIC_VERSION_DEFAULT

2017-05-15 Thread Andrea Bolognani
The QEMU default is GICv2, and some of the code in libvirt
relies on the exact value. Stop pretending that's not the
case and use GICv2 explicitly where needed.

Signed-off-by: Andrea Bolognani 
---
 src/qemu/qemu_command.c | 6 +++---
 src/qemu/qemu_domain.c  | 7 +++
 src/util/virgic.h   | 3 ---
 3 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 813a851..c2a9415 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7374,9 +7374,9 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
 goto cleanup;
 }
 
-/* The default GIC version should not be specified on the
- * QEMU commandline for backwards compatibility reasons */
-if (def->gic_version != VIR_GIC_VERSION_DEFAULT) {
+/* The default GIC version (GICv2) should not be specified on
+ * the QEMU commandline for backwards compatibility reasons */
+if (def->gic_version != VIR_GIC_VERSION_2) {
 if (!virQEMUCapsGet(qemuCaps,
 QEMU_CAPS_MACH_VIRT_GIC_VERSION)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 079a134..fdffe17 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2596,12 +2596,11 @@ qemuDomainDefEnableDefaultFeatures(virDomainDefPtr def,
 def->features[VIR_DOMAIN_FEATURE_GIC] = VIR_TRISTATE_SWITCH_ON;
 }
 
-/* Use the default GIC version if no version was specified */
+/* Use the default GIC version (GICv2) if no version was specified */
 if (def->features[VIR_DOMAIN_FEATURE_GIC] == VIR_TRISTATE_SWITCH_ON &&
 def->gic_version == VIR_GIC_VERSION_NONE) {
-VIR_DEBUG("Using GIC version %s (default)",
-  virGICVersionTypeToString(VIR_GIC_VERSION_DEFAULT));
-def->gic_version = VIR_GIC_VERSION_DEFAULT;
+VIR_DEBUG("Using GIC version 2 (default)");
+def->gic_version = VIR_GIC_VERSION_2;
 }
 }
 
diff --git a/src/util/virgic.h b/src/util/virgic.h
index 1c9efd6..2d77fdd 100644
--- a/src/util/virgic.h
+++ b/src/util/virgic.h
@@ -35,9 +35,6 @@ typedef enum {
 
 VIR_ENUM_DECL(virGICVersion);
 
-/* Consider GIC v2 the default */
-# define VIR_GIC_VERSION_DEFAULT VIR_GIC_VERSION_2
-
 typedef enum {
 VIR_GIC_IMPLEMENTATION_NONE = 0,
 VIR_GIC_IMPLEMENTATION_KERNEL = (1 << 1),
-- 
2.7.4

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


[libvirt] [PATCH v2 5/5] news: Update for GIC version on TCG changes

2017-05-15 Thread Andrea Bolognani
Signed-off-by: Andrea Bolognani 
---
 docs/news.xml | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index 2f01449..4cf14b0 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -37,6 +37,17 @@
 
 
 
+  
+
+  qemu: Use GICv2 by default for aarch64/virt TCG guests
+
+
+  The emulated GICv3 has some limitations that make it unusable as a
+  default; use GICv2 until they're sorted out. This change makes it
+  once again possible to run aarch64/virt guests on a x86_64 host
+  without having to tweak their configuration.
+
+  
 
 
 
-- 
2.7.4

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


[libvirt] [PATCH v2 2/5] tests: Check default GIC version for aarch64/virt TCG guests

2017-05-15 Thread Andrea Bolognani
Signed-off-by: Andrea Bolognani 
---
 .../qemuxml2argv-aarch64-gic-none-tcg.args | 19 
 .../qemuxml2argv-aarch64-gic-none-tcg.xml  | 17 +++
 tests/qemuxml2argvtest.c   |  3 +++
 .../qemuxml2xmlout-aarch64-gic-none-tcg.xml| 25 ++
 tests/qemuxml2xmltest.c|  1 +
 5 files changed, 65 insertions(+)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-gic-none-tcg.xml

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.args 
b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.args
new file mode 100644
index 000..975a014
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.args
@@ -0,0 +1,19 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-aarch64 \
+-name guest \
+-S \
+-machine virt,accel=tcg,gic-version=3 \
+-cpu cortex-a57 \
+-m 1024 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid 6ba410c5-1e5c-4d57-bee7-2228e7ffa32f \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-guest/monitor.sock,server,nowait \
+-no-acpi \
+-boot c
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.xml
new file mode 100644
index 000..0aa33db
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.xml
@@ -0,0 +1,17 @@
+
+  guest
+  6ba410c5-1e5c-4d57-bee7-2228e7ffa32f
+  1048576
+  1
+  
+hvm
+
+  
+  
+cortex-a57
+  
+  
+/usr/bin/qemu-system-aarch64
+
+  
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index ce87938..8273725 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2251,6 +2251,9 @@ mymain(void)
 DO_TEST_GIC("aarch64-gic-none-both", GIC_BOTH,
 QEMU_CAPS_KVM, QEMU_CAPS_MACHINE_OPT,
 QEMU_CAPS_MACH_VIRT_GIC_VERSION);
+DO_TEST_GIC("aarch64-gic-none-tcg", GIC_BOTH,
+QEMU_CAPS_MACHINE_OPT,
+QEMU_CAPS_MACH_VIRT_GIC_VERSION);
 DO_TEST_GIC("aarch64-gic-default", GIC_NONE,
 QEMU_CAPS_KVM, QEMU_CAPS_MACHINE_OPT);
 DO_TEST_GIC("aarch64-gic-default", GIC_NONE,
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-gic-none-tcg.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-gic-none-tcg.xml
new file mode 100644
index 000..69510e2
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-gic-none-tcg.xml
@@ -0,0 +1,25 @@
+
+  guest
+  6ba410c5-1e5c-4d57-bee7-2228e7ffa32f
+  1048576
+  1048576
+  1
+  
+hvm
+
+  
+  
+
+  
+  
+cortex-a57
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-aarch64
+
+  
+
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 2dccde7..c75199e 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1070,6 +1070,7 @@ mymain(void)
 DO_TEST_FULL("aarch64-gic-none-v2", WHEN_BOTH, GIC_V2, NONE);
 DO_TEST_FULL("aarch64-gic-none-v3", WHEN_BOTH, GIC_V3, NONE);
 DO_TEST_FULL("aarch64-gic-none-both", WHEN_BOTH, GIC_BOTH, NONE);
+DO_TEST_FULL("aarch64-gic-none-tcg", WHEN_BOTH, GIC_BOTH, NONE);
 DO_TEST_FULL("aarch64-gic-default", WHEN_BOTH, GIC_NONE, NONE);
 DO_TEST_FULL("aarch64-gic-default", WHEN_BOTH, GIC_V2, NONE);
 DO_TEST_FULL("aarch64-gic-default", WHEN_BOTH, GIC_V3, NONE);
-- 
2.7.4

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


[libvirt] [PATCH v2 1/5] qemu: Use qemuDomainMachineIsVirt() more

2017-05-15 Thread Andrea Bolognani
Signed-off-by: Andrea Bolognani 
---
 src/qemu/qemu_capabilities.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 71951e6..cf4dc74 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -5824,12 +5824,7 @@ virQEMUCapsFillDomainFeatureGICCaps(virQEMUCapsPtr 
qemuCaps,
 virDomainCapsFeatureGICPtr gic = >gic;
 virGICVersion version;
 
-if (domCaps->arch != VIR_ARCH_ARMV7L &&
-domCaps->arch != VIR_ARCH_AARCH64)
-return 0;
-
-if (STRNEQ(domCaps->machine, "virt") &&
-!STRPREFIX(domCaps->machine, "virt-"))
+if (!qemuDomainMachineIsVirt(domCaps->machine, domCaps->arch))
 return 0;
 
 for (version = VIR_GIC_VERSION_LAST - 1;
-- 
2.7.4

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


Re: [libvirt] [PATCH 07/10] apparmor: include local apparmor profiles

2017-05-15 Thread Stefan Bader
On 15.05.2017 16:30, Jamie Strandboge wrote:
> On Mon, 2017-05-15 at 09:28 -0500, Jamie Strandboge wrote:
>> On Mon, 2017-05-15 at 15:23 +0200, Stefan Bader wrote:
>>> From: Felix Geyer 
>>>
>>> Local overrides is a feature Debian/Ubuntu libvirt provided for a while.
>>> This allows the user to have a non-conffile that he can use to extend the
>>> package delivered rules with extra content matching his special case.
>>>
>>> This change adds the include directives to the apparmor profiles
>>> for virt-aa-helper and libvirtd.
>>>
>>
>> I'm fine with this change but it is important to understand that
>> /etc/apparmor.d/local/usr.sbin.libvirtd must exist otherwise the profile will
>> fail to load. In Debian/Ubuntu we use dh_apparmor which takes care of this 
>> for
>> us. If this is upstreamed, then wherever install of the profile happens or is
>> documented, then the local changes file needs to also be 
>> installed/documented.
>> Other non-deb distributions might not like this extra file, so it is possible
>> this may be a Debian and its derivatives thing
>>
> 
> Oh heh, I see you adjusted the Makefile.am for this in 08. Thanks!

Yeah, I guess it could make sense to merge those two changes into one. I was
just hesitating initially as the first part came via Debian and the latter is
and extension I did. Admittedly it is not completely consistent as I did merge
for other things.

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




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 07/10] apparmor: include local apparmor profiles

2017-05-15 Thread Jamie Strandboge
On Mon, 2017-05-15 at 09:28 -0500, Jamie Strandboge wrote:
> On Mon, 2017-05-15 at 15:23 +0200, Stefan Bader wrote:
> > From: Felix Geyer 
> > 
> > Local overrides is a feature Debian/Ubuntu libvirt provided for a while.
> > This allows the user to have a non-conffile that he can use to extend the
> > package delivered rules with extra content matching his special case.
> > 
> > This change adds the include directives to the apparmor profiles
> > for virt-aa-helper and libvirtd.
> > 
> 
> I'm fine with this change but it is important to understand that
> /etc/apparmor.d/local/usr.sbin.libvirtd must exist otherwise the profile will
> fail to load. In Debian/Ubuntu we use dh_apparmor which takes care of this for
> us. If this is upstreamed, then wherever install of the profile happens or is
> documented, then the local changes file needs to also be installed/documented.
> Other non-deb distributions might not like this extra file, so it is possible
> this may be a Debian and its derivatives thing
> 

Oh heh, I see you adjusted the Makefile.am for this in 08. Thanks!

-- 
Jamie Strandboge | http://www.canonical.com

signature.asc
Description: This is a digitally signed message part
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 2/2] qemu: don't relabel chardev source file if virtlogd is used

2017-05-15 Thread Pavel Hrdina
If libvirt uses virtlogd instead of passing the file path directly
to QEMU we shouldn't relabel the chardev source file, otherwise
virtlogd will get a permission denied while reloading.

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

Signed-off-by: Pavel Hrdina 
---
 src/conf/domain_conf.c  | 20 
 src/conf/domain_conf.h  |  1 +
 src/qemu/qemu_command.c | 12 
 src/security/security_dac.c |  6 ++
 src/security/security_selinux.c |  6 ++
 5 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index aa441fae3c..92f011d3a4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2064,6 +2064,7 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest,
 }
 
 dest->type = src->type;
+dest->skipRelabel = src->skipRelabel;
 
 return 0;
 }
@@ -10608,6 +10609,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr 
def,
 char *append = NULL;
 char *haveTLS = NULL;
 char *tlsFromConfig = NULL;
+char *skipRelabel = NULL;
 int remaining = 0;
 
 while (cur != NULL) {
@@ -10628,6 +10630,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr 
def,
 case VIR_DOMAIN_CHR_TYPE_UNIX:
 if (!append && def->type == VIR_DOMAIN_CHR_TYPE_FILE)
 append = virXMLPropString(cur, "append");
+if (!skipRelabel && def->type == VIR_DOMAIN_CHR_TYPE_FILE)
+skipRelabel = virXMLPropString(cur, "skipRelabel");
 /* PTY path is only parsed from live xml.  */
 if (!path  &&
 (def->type != VIR_DOMAIN_CHR_TYPE_PTY ||
@@ -10726,6 +10730,17 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr 
def,
_("Invalid append attribute value '%s'"), append);
 goto error;
 }
+if (skipRelabel && def->type == VIR_DOMAIN_CHR_TYPE_FILE &&
+(flags & VIR_DOMAIN_DEF_PARSE_STATUS)) {
+if (STREQ(skipRelabel, "yes")) {
+def->skipRelabel = true;
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("invalid 'skipRelabel' attribute value '%s'"),
+   skipRelabel);
+goto error;
+}
+}
 if (!path &&
 def->type != VIR_DOMAIN_CHR_TYPE_PTY) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -10902,6 +10917,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr 
def,
 VIR_FREE(logfile);
 VIR_FREE(haveTLS);
 VIR_FREE(tlsFromConfig);
+VIR_FREE(skipRelabel);
 
 return remaining;
 
@@ -22324,6 +22340,10 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
 def->data.file.append != VIR_TRISTATE_SWITCH_ABSENT)
 virBufferAsprintf(buf, " append='%s'",
 virTristateSwitchTypeToString(def->data.file.append));
+if ((flags & VIR_DOMAIN_DEF_FORMAT_STATUS) &&
+def->type == VIR_DOMAIN_CHR_TYPE_FILE && def->skipRelabel) {
+virBufferAddLit(buf, " skipRelabel='yes'");
+}
 virDomainSourceDefFormatSeclabel(buf, nseclabels, seclabels, 
flags);
 }
 break;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 09fb7aada4..329eb90392 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1166,6 +1166,7 @@ struct _virDomainChrSourceDef {
 } data;
 char *logfile;
 int logappend;
+bool skipRelabel;
 };
 
 /* A complete character device, both host and domain views.  */
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 813a8515c0..0625075bb2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4998,6 +4998,7 @@ static int
 qemuBuildChrChardevFileStr(virLogManagerPtr logManager,
virCommandPtr cmd,
const virDomainDef *def,
+   virDomainChrSourceDefPtr sourceDef,
virBufferPtr buf,
const char *filearg, const char *fileval,
const char *appendarg, int appendval)
@@ -5011,6 +5012,9 @@ qemuBuildChrChardevFileStr(virLogManagerPtr logManager,
 appendval == VIR_TRISTATE_SWITCH_OFF)
 flags |= VIR_LOG_MANAGER_PROTOCOL_DOMAIN_OPEN_LOG_FILE_TRUNCATE;
 
+if (sourceDef)
+sourceDef->skipRelabel = true;
+
 if ((logfd = virLogManagerDomainOpenLogFile(logManager,
 "qemu",
 def->uuid,
@@ -5051,7 +5055,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
virCommandPtr cmd,
virQEMUDriverConfigPtr cfg,
   

Re: [libvirt] [PATCH 07/10] apparmor: include local apparmor profiles

2017-05-15 Thread Jamie Strandboge
On Mon, 2017-05-15 at 15:23 +0200, Stefan Bader wrote:
> From: Felix Geyer 
> 
> Local overrides is a feature Debian/Ubuntu libvirt provided for a while.
> This allows the user to have a non-conffile that he can use to extend the
> package delivered rules with extra content matching his special case.
> 
> This change adds the include directives to the apparmor profiles
> for virt-aa-helper and libvirtd.
> 

I'm fine with this change but it is important to understand that
/etc/apparmor.d/local/usr.sbin.libvirtd must exist otherwise the profile will
fail to load. In Debian/Ubuntu we use dh_apparmor which takes care of this for
us. If this is upstreamed, then wherever install of the profile happens or is
documented, then the local changes file needs to also be installed/documented.
Other non-deb distributions might not like this extra file, so it is possible
this may be a Debian and its derivatives thing

> Signed-off-by: Christian Ehrhardt 
> Signed-off-by: Stefan Bader 
> ---
>  examples/apparmor/usr.lib.libvirt.virt-aa-helper | 3 +++
>  examples/apparmor/usr.sbin.libvirtd  | 3 +++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/examples/apparmor/usr.lib.libvirt.virt-aa-helper
> b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
> index 012080c..93ba74e 100644
> --- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper
> +++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
> @@ -56,4 +56,7 @@ profile virt-aa-helper /usr/{lib,lib64}/libvirt/virt-aa-
> helper {
>    /**.vmdk r,
>    /**.[iI][sS][oO] r,
>    /**/disk{,.*} r,
> +
> +  # Site-specific additions and overrides. See local/README for details.
> +  #include 
>  }
> diff --git a/examples/apparmor/usr.sbin.libvirtd
> b/examples/apparmor/usr.sbin.libvirtd
> index 353b039..c37d5ee 100644
> --- a/examples/apparmor/usr.sbin.libvirtd
> +++ b/examples/apparmor/usr.sbin.libvirtd
> @@ -85,4 +85,7 @@
>  
> /usr/{lib,lib64,lib/qemu,libexec}/qemu-bridge-helper rmix,
>    }
> +
> +  # Site-specific additions and overrides. See local/README for details.
> +  #include 
>  }
-- 
Jamie Strandboge | http://www.canonical.com

signature.asc
Description: This is a digitally signed message part
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 0/2] fix labeling for chardev source path

2017-05-15 Thread Pavel Hrdina
Pavel Hrdina (2):
  conf: don't iterate over backcompat console in virDomainChrDefForeach
  qemu: don't relabel chardev source file if virtlogd is used

 src/conf/domain_conf.c  | 46 -
 src/conf/domain_conf.h  |  1 +
 src/qemu/qemu_command.c | 12 +++
 src/security/security_dac.c |  6 ++
 src/security/security_selinux.c | 16 ++
 5 files changed, 62 insertions(+), 19 deletions(-)

-- 
2.13.0

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


[libvirt] [PATCH 1/2] conf: don't iterate over backcompat console in virDomainChrDefForeach

2017-05-15 Thread Pavel Hrdina
If the first console is just a copy of the first serial device we
don't need to iterate over the same device twice in order to perform
actions like security labeling, cgroup configuring, etc.

Currently only security SELinux manager was aware of this fact.

Signed-off-by: Pavel Hrdina 
---
 src/conf/domain_conf.c  | 26 +-
 src/security/security_selinux.c | 10 --
 2 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0ff216e3a3..aa441fae3c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3517,6 +3517,24 @@ void virDomainDeviceInfoClear(virDomainDeviceInfoPtr 
info)
 }
 
 
+static bool
+virDomainSkipBackcompatConsole(virDomainDefPtr def,
+   size_t index,
+   bool all)
+{
+virDomainChrDefPtr console = def->consoles[index];
+
+if (!all && index == 0 &&
+(console->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL ||
+ console->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE) &&
+def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
+return true;
+}
+
+return false;
+}
+
+
 static int
 virDomainDeviceInfoIterateInternal(virDomainDefPtr def,
virDomainDeviceInfoCallback cb,
@@ -3585,11 +3603,7 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def,
 return -1;
 }
 for (i = 0; i < def->nconsoles; i++) {
-if (!all &&
-i == 0 &&
-(def->consoles[i]->targetType == 
VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL ||
- def->consoles[i]->targetType == 
VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_NONE) &&
- def->os.type == VIR_DOMAIN_OSTYPE_HVM)
+if (virDomainSkipBackcompatConsole(def, i, all))
 continue;
 device.data.chr = def->consoles[i];
 if (cb(def, , >consoles[i]->info, opaque) < 0)
@@ -25313,6 +25327,8 @@ virDomainChrDefForeach(virDomainDefPtr def,
 goto done;
 }
 for (i = 0; i < def->nconsoles; i++) {
+if (virDomainSkipBackcompatConsole(def, i, false))
+continue;
 if ((iter)(def,
def->consoles[i],
opaque) < 0)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index df7c96833e..612dbc2a83 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -2339,11 +2339,6 @@ 
virSecuritySELinuxRestoreSecurityChardevCallback(virDomainDefPtr def,
 {
 virSecurityManagerPtr mgr = opaque;
 
-/* This is taken care of by processing of def->serials */
-if (dev->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE &&
-dev->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL)
-return 0;
-
 return virSecuritySELinuxRestoreChardevLabel(mgr, def, dev, dev->source);
 }
 
@@ -2733,11 +2728,6 @@ 
virSecuritySELinuxSetSecurityChardevCallback(virDomainDefPtr def,
 {
 virSecurityManagerPtr mgr = opaque;
 
-/* This is taken care of by processing of def->serials */
-if (dev->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE &&
-dev->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL)
-return 0;
-
 return virSecuritySELinuxSetChardevLabel(mgr, def, dev, dev->source);
 }
 
-- 
2.13.0

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


[libvirt] [PATCH] vz: support virDomainGetBlockInfo in driver

2017-05-15 Thread Nikolay Shirokovskiy
Actually physical size is not available in vz sdk right now so let's
set it to allocation as an estimation in non sparse case.
---
 src/vz/vz_driver.c | 50 ++
 src/vz/vz_sdk.c| 23 +++
 src/vz/vz_sdk.h|  1 +
 3 files changed, 74 insertions(+)

diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 9a429f4..7b32558 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -4005,6 +4005,55 @@ vzDomainBlockResize(virDomainPtr domain,
 return ret;
 }
 
+static int
+vzDomainGetBlockInfo(virDomainPtr domain,
+ const char *path,
+ virDomainBlockInfoPtr info,
+ unsigned int flags)
+{
+virDomainObjPtr dom;
+virDomainDiskDefPtr disk;
+long long allocation;
+bool job = false;
+int ret = -1;
+
+virCheckFlags(0, -1);
+
+if (!(dom = vzDomObjFromDomainRef(domain)))
+return -1;
+
+if (virDomainGetBlockInfoEnsureACL(domain->conn, dom->def) < 0)
+goto cleanup;
+
+if (vzDomainObjBeginJob(dom) < 0)
+goto cleanup;
+job = true;
+
+if (vzEnsureDomainExists(dom) < 0)
+goto cleanup;
+
+if (!(disk = virDomainDiskByName(dom->def, path, false))) {
+virReportError(VIR_ERR_INVALID_ARG,
+   _("invalid path %s not assigned to domain"), path);
+goto cleanup;
+}
+
+if ((allocation = prlsdkGetDiskAllocation(dom, disk)) < 0)
+goto cleanup;
+
+info->capacity = disk->src->capacity;
+info->allocation = allocation;
+info->physical = allocation;
+
+ret = 0;
+
+ cleanup:
+if (job)
+vzDomainObjEndJob(dom);
+virDomainObjEndAPI();
+return ret;
+}
+
 static virHypervisorDriver vzHypervisorDriver = {
 .name = "vz",
 .connectOpen = vzConnectOpen,/* 0.10.0 */
@@ -4106,6 +4155,7 @@ static virHypervisorDriver vzHypervisorDriver = {
 .domainAbortJob = vzDomainAbortJob, /* 3.1.0 */
 .domainReset = vzDomainReset, /* 3.1.0 */
 .domainBlockResize = vzDomainBlockResize, /* 3.3.0 */
+.domainGetBlockInfo = vzDomainGetBlockInfo, /* 3.4.0 */
 };
 
 static virConnectDriver vzConnectDriver = {
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 4d2c6b0..b8f561c 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -4962,3 +4962,26 @@ int prlsdkResizeImage(virDomainObjPtr dom, 
virDomainDiskDefPtr disk,
 PrlHandle_Free(prldisk);
 return ret;
 }
+
+long long prlsdkGetDiskAllocation(virDomainObjPtr dom,
+  virDomainDiskDefPtr disk)
+{
+vzDomObjPtr privdom = dom->privateData;
+PRL_HANDLE job;
+PRL_HANDLE prldisk;
+PRL_UINT32 size;
+PRL_RESULT pret;
+
+job = PrlVm_RefreshConfig(privdom->sdkdom);
+if (waitDomainJob(job, dom))
+return -1;
+
+prldisk = prlsdkGetDisk(privdom->sdkdom, disk);
+if (prldisk == PRL_INVALID_HANDLE)
+return -1;
+
+pret = PrlVmDevHd_GetSizeOnDisk(prldisk, );
+prlsdkCheckRetExit(pret, -1);
+
+return ((unsigned long long)size) << 20;
+}
diff --git a/src/vz/vz_sdk.h b/src/vz/vz_sdk.h
index 0a77431..6b73067 100644
--- a/src/vz/vz_sdk.h
+++ b/src/vz/vz_sdk.h
@@ -91,3 +91,4 @@ PRL_HANDLE
 prlsdkSdkDomainLookupByName(vzDriverPtr driver, const char *name);
 int prlsdkCancelJob(virDomainObjPtr dom);
 int prlsdkResizeImage(virDomainObjPtr dom, virDomainDiskDefPtr disk, unsigned 
long long newsize);
+long long prlsdkGetDiskAllocation(virDomainObjPtr dom, virDomainDiskDefPtr 
disk);
-- 
1.8.3.1

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


Re: [libvirt] [PATCH 3/5] qemu: Use GICv2 for aarch64/virt TCG guests

2017-05-15 Thread Andrea Bolognani
On Mon, 2017-05-15 at 12:53 +0200, Peter Krempa wrote:
> > +/* We want to use the highest available GIC version for guests;
> > + * however, the emulated GICv3 is currently lacking a MSI 
> > controller,
> > + * making it unsuitable for the pure PCIe topology we aim for.
> > + *
> > + * For that reason, we skip this step entirely for TCG guests,
> > + * and rely on the code below to pick the default version, GICv2,
> > + * which supports all the features we need.
> > + *
> > + * We'll want to revisit this once MSI support for GICv3 has been
> > + * implemented in QEMU.
> > + *
> > + * See https://bugzilla.redhat.com/show_bug.cgi?id=1414081 */
> > +if (def->virtType == VIR_DOMAIN_VIRT_KVM) {
> 
> Currently it does not matter that much, since there are only two
> versions but this looks very non-future-proof to me.
> 
> When qemu adds the feature you'll need to add a capability, where you
> also enable the code below for TCG guests.
> 
> If there will be another version or something the condition will need to
> be altered.
> 
> I'd rather see that v3 is specifically disqualified for TCG guests
> (which will be later relaxed using the capability.). That way you'll
> still run the detection process.

Can do.

I'll post a respin shortly.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH] Add support for VNC autoport feature for bhyve hypervisor.

2017-05-15 Thread Roman Bogorodskiy
  Alexander Nusov wrote:

>  On Wed, 10 May 2017 17:58:05 +0300 Roman Bogorodskiy 
> bogorods...@gmail.com wrote 
> 
> 
> 
> Alexander Nusov wrote: 
> 
> 
> 
>  This patch adds support for automatic VNC port assignment for bhyve 
> guests. 
> 
>  
> 
>  --- 
> 
>  src/bhyve/bhyve_command.c | 9 + 
> 
>  src/bhyve/bhyve_driver.c | 5 + 
> 
>  src/bhyve/bhyve_utils.h | 3 +++ 
> 
>  3 files changed, 17 insertions(+) 
> 
> 
> 
> Hi Alexander, 
> 
> 
> 
> Thanks for implementing this! Overall it looks good, two comments: 
> 
> 
> 
> * It doesn't take into account domains that use VNC with port 
> 
> explicitly specified. For example, if I start a domain with VNC 
> 
> configuration "port=5900 autoport=no", I will not be able 
> 
> to start a domain with "autoport=yes" because it will try to 
> 
> to 5900 and will fail to bind. 
> 
> 
> 
> Looking at the qemu driver code, it seems it's done by calling 
> 
> virPortAllocatorSetUsed(). 
> 
> 
> 
> 
> 
> Hi Roman. Thanks for finding this. I tried adding virPortAllocatorSetUsed() 
> call but it seems it should be 
> 
> placed somewhere at the initialization of libvirtd driver also in case of 
> restaring the libvirtd daemon.
> 

Yeah, that sounds right.

FWIW, the bhyve driver tries to reconnect to domains after restart, you
can check virBhyveProcessReconnectAll() in bhyve_process.c.

> 
> 
> 
> I'm also wondering how it's handled for autostarting domains, e.g. 
> 
> if I have vm1[5900], vm2[5901], vm3[autoport], vm4[autoport], 
> 
> they'll start fine in that order, but will fail to start if going 
> 
> this way: vm3, vm4, vm1, vm2 
> 
> 
> 
> 
> 
> To be honest, I'm concerned also.
> 
> 
> 
> https://www.redhat.com/archives/libvir-list/2011-April/msg00819.html
> 
> is that still actual?
> 

Daniel P. Berrange suggested on IRC that specifying autostart order is not
supported and it's actually not desired to mix manual port assignment
and autoport on the same host.

Also, qemu supports obtaining autoport range from the config file, we'd
probably want to have a similar behaviour for the bhyve driver (but as a
separate patch I guess).

Also, Daniel noted that this patch needs to add
virPortAllocatorRelease() when domain goes down.

> 
> 
> 
> * Nitpick: Commit message titles are usually don't end with a period. 
> 
> 
> Also, they're usually prefixed with the relevant subsystem, so 
> 
> this one could be "bhyve: Add support for VNC autoport feature" 
> 
> for example (you might glance through 'git log' for examples.
> 
> 
> 
> 
> good.
> 
> 
> 
> 

Roman Bogorodskiy


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

[libvirt] [PATCH 10/10] apparmor, libvirt-qemu: Add ppc related changes

2017-05-15 Thread Stefan Bader
From: Serge Hallyn 

Updates profile to allow running on ppc64el.

Bug-Ubuntu: https://bugs.launchpad.net/bugs/1374554

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
---
 examples/apparmor/libvirt-qemu | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
index f04ce04..2791dfc 100644
--- a/examples/apparmor/libvirt-qemu
+++ b/examples/apparmor/libvirt-qemu
@@ -77,6 +77,7 @@
   /usr/share/OVMF/** r,
   /usr/share/AAVMF/** r,
   /usr/share/qemu-efi/** r,
+  /usr/share/slof/** r,
 
   # access PKI infrastructure
   /etc/pki/libvirt-vnc/** r,
@@ -102,6 +103,7 @@
   /usr/bin/qemu-system-or32 rmix,
   /usr/bin/qemu-system-ppc rmix,
   /usr/bin/qemu-system-ppc64 rmix,
+  /usr/bin/qemu-system-ppc64le rmix,
   /usr/bin/qemu-system-ppcemb rmix,
   /usr/bin/qemu-system-s390x rmix,
   /usr/bin/qemu-system-sh4 rmix,
@@ -158,3 +160,8 @@
   /etc/udev/udev.conf r,
   /sys/bus/ r,
   /sys/class/ r,
+
+  # for ppc device-tree access
+  @{PROC}/device-tree/ r,
+  @{PROC}/device-tree/** r,
+  /sys/firmware/devicetree/** r,
-- 
2.7.4

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


[libvirt] [PATCH 02/10] apparmor, virt-aa-helper: allow /usr/share/OVMF/ too

2017-05-15 Thread Stefan Bader
From: Simon McVittie 

The split firmware and variables files introduced by
https://bugs.debian.org/764918 are in a different directory for some reason.
Let the virtual machine read both.

Extended by Christian Ehrhardt to generalize FW test (simplifies
additional testing on firmware files in future).

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
---
 examples/apparmor/libvirt-qemu |  1 +
 src/security/virt-aa-helper.c  |  1 +
 tests/virt-aa-helper-test  | 24 
 3 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
index a9020aa..e0988bb 100644
--- a/examples/apparmor/libvirt-qemu
+++ b/examples/apparmor/libvirt-qemu
@@ -70,6 +70,7 @@
   /usr/share/vgabios/** r,
   /usr/share/seabios/** r,
   /usr/share/ovmf/** r,
+  /usr/share/OVMF/** r,
 
   # access PKI infrastructure
   /etc/pki/libvirt-vnc/** r,
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index d976a00..dd166c2 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -512,6 +512,7 @@ valid_path(const char *path, const bool readonly)
 "/vmlinuz",
 "/initrd",
 "/initrd.img",
+"/usr/share/OVMF/",  /* for OVMF images */
 "/usr/share/ovmf/"   /* for OVMF images */
 };
 /* override the above with these */
diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test
index 68e9399..73f3080 100755
--- a/tests/virt-aa-helper-test
+++ b/tests/virt-aa-helper-test
@@ -145,6 +145,20 @@ testme() {
 fi
 }
 
+testfw() {
+title="$1"
+fwpath="$2"
+
+if [ -f "$fwpath" ]; then
+sed -e "s,###UUID###,$uuid,g"  \
+-e "s,###DISK###,$disk1,g" \
+-e "s,,$fwpath,g" "$template_xml" > "$test_xml"
+testme "0" "$title" "-r -u $valid_uuid" "$test_xml"
+else
+echo "Skipping FW $title test. Could not find $fwpath"
+fi
+}
+
 # Expected failures
 echo "Expected failures:" >$output
 testme "1" "invalid arg" "-z"
@@ -291,14 +305,8 @@ sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" 
-e "s,,$tm
 touch "$tmpdir/kernel"
 testme "0" "kernel" "-r -u $valid_uuid" "$test_xml"
 
-if [ -f /usr/share/ovmf/OVMF.fd ]; then
-sed -e "s,###UUID###,$uuid,g"  \
--e "s,###DISK###,$disk1,g" \
--e "s,,/usr/share/ovmf/OVMF.fd,g" "$template_xml" > 
"$test_xml"
-testme "0" "ovmf" "-r -u $valid_uuid" "$test_xml"
-else
-echo "Skipping OVMF test. Could not find /usr/share/ovmf/OVMF.fd"
-fi
+testfw "ovmf (old path)" "/usr/share/ovmf/OVMF.fd"
+testfw "OVMF (new path)" "/usr/share/OVMF/OVMF_CODE.fd"
 
 sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e 
"s,,$tmpdir/initrd,g" "$template_xml" > "$test_xml"
 touch "$tmpdir/initrd"
-- 
2.7.4

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


[libvirt] [PATCH 08/10] apparmor: provide local override templates

2017-05-15 Thread Stefan Bader
Local overrides is a feature Debian/Ubuntu libvirt provided for a while.
This allows the user to have a non-conffile that he can use to extend the
package delivered rules with extra content matching his special case.

This change provides override templates which the user can extend
and modifies the makefile template to include those when installing
the apparmor profiles.

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
---
 examples/Makefile.am   | 14 ++
 examples/apparmor/local-usr.lib.libvirt.virt-aa-helper |  2 ++
 examples/apparmor/local-usr.sbin.libvirtd  |  2 ++
 3 files changed, 18 insertions(+)
 create mode 100644 examples/apparmor/local-usr.lib.libvirt.virt-aa-helper
 create mode 100644 examples/apparmor/local-usr.sbin.libvirtd

diff --git a/examples/Makefile.am b/examples/Makefile.am
index 2956e14..16c7bf6 100644
--- a/examples/Makefile.am
+++ b/examples/Makefile.am
@@ -25,6 +25,8 @@ EXTRA_DIST = \
apparmor/libvirt-lxc \
apparmor/usr.lib.libvirt.virt-aa-helper \
apparmor/usr.sbin.libvirtd \
+   apparmor/local-usr.sbin.libvirtd \
+   apparmor/local-usr.lib.libvirt.virt-aa-helper \
lxcconvert/virt-lxc-convert \
polkit/libvirt-acl.rules \
$(wildcard $(srcdir)/systemtap/*.stp) \
@@ -74,6 +76,18 @@ apparmor_DATA = \
apparmor/usr.sbin.libvirtd \
$(NULL)
 
+localdir = $(apparmordir)/local
+local_DATA = \
+   apparmor/local-usr.sbin.libvirtd \
+   apparmor/local-usr.lib.libvirt.virt-aa-helper \
+   $(NULL)
+
+install-data-hook:
+   mv $(DESTDIR)$(localdir)/local-usr.sbin.libvirtd \
+  $(DESTDIR)$(localdir)/usr.sbin.libvirtd
+   mv $(DESTDIR)$(localdir)/local-usr.lib.libvirt.virt-aa-helper \
+  $(DESTDIR)$(localdir)/usr.lib.libvirt.virt-aa-helper
+
 abstractionsdir = $(apparmordir)/abstractions
 abstractions_DATA = \
apparmor/libvirt-qemu \
diff --git a/examples/apparmor/local-usr.lib.libvirt.virt-aa-helper 
b/examples/apparmor/local-usr.lib.libvirt.virt-aa-helper
new file mode 100644
index 000..82c9c39
--- /dev/null
+++ b/examples/apparmor/local-usr.lib.libvirt.virt-aa-helper
@@ -0,0 +1,2 @@
+# Site-specific additions and overrides for usr.lib.libvirt.virt-aa-helper.
+# For more details, please see /etc/apparmor.d/local/README.
diff --git a/examples/apparmor/local-usr.sbin.libvirtd 
b/examples/apparmor/local-usr.sbin.libvirtd
new file mode 100644
index 000..6e19f20
--- /dev/null
+++ b/examples/apparmor/local-usr.sbin.libvirtd
@@ -0,0 +1,2 @@
+# Site-specific additions and overrides for usr.sbin.libvirtd.
+# For more details, please see /etc/apparmor.d/local/README.
-- 
2.7.4

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


[libvirt] [PATCH 03/10] apparmor, virt-aa-helper: Allow aarch64 UEFI.

2017-05-15 Thread Stefan Bader
From: William Grant 

Allow access to aarch64 UEFI images.

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
---
 examples/apparmor/libvirt-qemu | 2 ++
 src/security/virt-aa-helper.c  | 4 +++-
 tests/virt-aa-helper-test  | 2 ++
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
index e0988bb..89466c9 100644
--- a/examples/apparmor/libvirt-qemu
+++ b/examples/apparmor/libvirt-qemu
@@ -71,6 +71,8 @@
   /usr/share/seabios/** r,
   /usr/share/ovmf/** r,
   /usr/share/OVMF/** r,
+  /usr/share/AAVMF/** r,
+  /usr/share/qemu-efi/** r,
 
   # access PKI infrastructure
   /etc/pki/libvirt-vnc/** r,
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index dd166c2..a2d5c21 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -513,7 +513,9 @@ valid_path(const char *path, const bool readonly)
 "/initrd",
 "/initrd.img",
 "/usr/share/OVMF/",  /* for OVMF images */
-"/usr/share/ovmf/"   /* for OVMF images */
+"/usr/share/ovmf/",  /* for OVMF images */
+"/usr/share/AAVMF/", /* for AAVMF images */
+"/usr/share/qemu-efi/"   /* for AAVMF images */
 };
 /* override the above with these */
 const char * const override[] = {
diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test
index 73f3080..51072f6 100755
--- a/tests/virt-aa-helper-test
+++ b/tests/virt-aa-helper-test
@@ -307,6 +307,8 @@ testme "0" "kernel" "-r -u $valid_uuid" "$test_xml"
 
 testfw "ovmf (old path)" "/usr/share/ovmf/OVMF.fd"
 testfw "OVMF (new path)" "/usr/share/OVMF/OVMF_CODE.fd"
+testfw "AAVMF" "/usr/share/AAVMF/AAVMF_CODE.fd"
+testfw "qemu-efi" "/usr/share/qemu-efi/QEMU_EFI.fd"
 
 sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e 
"s,,$tmpdir/initrd,g" "$template_xml" > "$test_xml"
 touch "$tmpdir/initrd"
-- 
2.7.4

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


[libvirt] [PATCH 09/10] appmor, virt-aa-helper: Add 9p support

2017-05-15 Thread Stefan Bader
From: Serge Hallyn 

Add fowner and fsetid to libvirt-qemu profile and add link
to 9p file options in virt-aa-helper.

Bug-Ubuntu: https://bugs.launchpad.net/bugs/1378434

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
---
 examples/apparmor/libvirt-qemu | 4 
 src/security/virt-aa-helper.c  | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
index 89466c9..f04ce04 100644
--- a/examples/apparmor/libvirt-qemu
+++ b/examples/apparmor/libvirt-qemu
@@ -13,6 +13,10 @@
   capability setgid,
   capability setuid,
 
+  # for 9p
+  capability fsetid,
+  capability fowner,
+
   network inet stream,
   network inet6 stream,
 
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index a2d5c21..667241b 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1108,7 +1108,7 @@ get_files(vahControl * ctl)
 /* We don't need to add deny rw rules for readonly mounts,
  * this can only lead to troubles when mounting / readonly.
  */
-if (vah_add_path(, fs->src->path, fs->readonly ? "R" : "rw", 
true) != 0)
+if (vah_add_path(, fs->src->path, fs->readonly ? "R" : "rwl", 
true) != 0)
 goto cleanup;
 }
 }
-- 
2.7.4

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


[libvirt] [PATCH 04/10] apparmor, virt-aa-helper: Allow access to libnl-3 config files

2017-05-15 Thread Stefan Bader
From: Felix Geyer 

Allow access to libnl-3 config files

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
---
 examples/apparmor/usr.lib.libvirt.virt-aa-helper | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/examples/apparmor/usr.lib.libvirt.virt-aa-helper 
b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
index 4a8f197..ee53c2c 100644
--- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper
+++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
@@ -16,6 +16,8 @@ profile virt-aa-helper 
/usr/{lib,lib64}/libvirt/virt-aa-helper {
   owner @{PROC}/[0-9]*/status r,
   @{PROC}/filesystems r,
 
+  /etc/libnl-3/classid r,
+
   # for hostdev
   /sys/devices/ r,
   /sys/devices/** r,
-- 
2.7.4

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


[libvirt] [PATCH 06/10] apparmor, virt-aa-helper: Additional explicit denies for host devices

2017-05-15 Thread Stefan Bader
From: Christian Ehrhardt 

This adds further explicit denies for host devices to silence
(acceptable) denial warnings.

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
---
 examples/apparmor/usr.lib.libvirt.virt-aa-helper | 4 
 1 file changed, 4 insertions(+)

diff --git a/examples/apparmor/usr.lib.libvirt.virt-aa-helper 
b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
index 7804b72..012080c 100644
--- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper
+++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
@@ -24,6 +24,10 @@ profile virt-aa-helper 
/usr/{lib,lib64}/libvirt/virt-aa-helper {
   deny /dev/sd* r,
   deny /dev/vd* r,
   deny /dev/dm-* r,
+  deny /dev/drbd[0-9]* r,
+  deny /dev/dasd* r,
+  deny /dev/nvme* r,
+  deny /dev/zd[0-9]* r,
   deny /dev/mapper/ r,
   deny /dev/mapper/* r,
 
-- 
2.7.4

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


[libvirt] [PATCH 07/10] apparmor: include local apparmor profiles

2017-05-15 Thread Stefan Bader
From: Felix Geyer 

Local overrides is a feature Debian/Ubuntu libvirt provided for a while.
This allows the user to have a non-conffile that he can use to extend the
package delivered rules with extra content matching his special case.

This change adds the include directives to the apparmor profiles
for virt-aa-helper and libvirtd.

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
---
 examples/apparmor/usr.lib.libvirt.virt-aa-helper | 3 +++
 examples/apparmor/usr.sbin.libvirtd  | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/examples/apparmor/usr.lib.libvirt.virt-aa-helper 
b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
index 012080c..93ba74e 100644
--- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper
+++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
@@ -56,4 +56,7 @@ profile virt-aa-helper 
/usr/{lib,lib64}/libvirt/virt-aa-helper {
   /**.vmdk r,
   /**.[iI][sS][oO] r,
   /**/disk{,.*} r,
+
+  # Site-specific additions and overrides. See local/README for details.
+  #include 
 }
diff --git a/examples/apparmor/usr.sbin.libvirtd 
b/examples/apparmor/usr.sbin.libvirtd
index 353b039..c37d5ee 100644
--- a/examples/apparmor/usr.sbin.libvirtd
+++ b/examples/apparmor/usr.sbin.libvirtd
@@ -85,4 +85,7 @@
 
/usr/{lib,lib64,lib/qemu,libexec}/qemu-bridge-helper rmix,
   }
+
+  # Site-specific additions and overrides. See local/README for details.
+  #include 
 }
-- 
2.7.4

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


[libvirt] [PATCH 01/10] virt-aa-helper: Ask for no deny rule for readonly disk elements

2017-05-15 Thread Stefan Bader
From: Serge Hallyn 

Just because a disk element only requests read access doesn't mean
there may not be another readwrite request.

Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/bugs/1554031

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
---
 src/security/virt-aa-helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 5f5d1cd..d976a00 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -888,11 +888,11 @@ add_file_path(virDomainDiskDefPtr disk,
 
 if (depth == 0) {
 if (disk->src->readonly)
-ret = vah_add_file(buf, path, "r");
+ret = vah_add_file(buf, path, "R");
 else
 ret = vah_add_file(buf, path, "rw");
 } else {
-ret = vah_add_file(buf, path, "r");
+ret = vah_add_file(buf, path, "R");
 }
 
 if (ret != 0)
-- 
2.7.4

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


[libvirt] [PATCH 05/10] apparmor, virt-aa-helper: Explicit denies for host devices

2017-05-15 Thread Stefan Bader
From: Felix Geyer 

Add explicit denies for disk devices to avoid cluttering dmesg with
(acceptable) denials.

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
---
 examples/apparmor/usr.lib.libvirt.virt-aa-helper | 5 +
 1 file changed, 5 insertions(+)

diff --git a/examples/apparmor/usr.lib.libvirt.virt-aa-helper 
b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
index ee53c2c..7804b72 100644
--- a/examples/apparmor/usr.lib.libvirt.virt-aa-helper
+++ b/examples/apparmor/usr.lib.libvirt.virt-aa-helper
@@ -21,6 +21,11 @@ profile virt-aa-helper 
/usr/{lib,lib64}/libvirt/virt-aa-helper {
   # for hostdev
   /sys/devices/ r,
   /sys/devices/** r,
+  deny /dev/sd* r,
+  deny /dev/vd* r,
+  deny /dev/dm-* r,
+  deny /dev/mapper/ r,
+  deny /dev/mapper/* r,
 
   /usr/{lib,lib64}/libvirt/virt-aa-helper mr,
   /{usr/,}sbin/apparmor_parser Ux,
-- 
2.7.4

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


[libvirt] Various apparmor related changes (part 1)

2017-05-15 Thread Stefan Bader
Over the years there have been a bunch of changes to the
apparmor profiles and/or virt-aa-helper which have been
carried in Debian/Ubuntu but never made it upstream.

In an attempt to clean this up and generally improve the
apparmor based environments, we (Christian and I) went
over the changes, cleaned out cruft as much as possible 
and would be sending out hunks of changes to this list
for upstream inclusion.

I hope doing multiple but smaller rounds of submissions
will make it simpler to get those reviewed and hopefully
accepted.

This first batch contains a mix of changes from Debian
and Ubuntu.

Thanks,
Stefan

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


Re: [libvirt] [PATCHv4 8/6] conf: add ABI stability checks for IOMMU options

2017-05-15 Thread Peter Krempa
On Fri, May 12, 2017 at 17:08:50 +0200, Ján Tomko wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1427005
> ---
>  src/conf/domain_conf.c | 26 ++
>  1 file changed, 26 insertions(+)

ACK


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

Re: [libvirt] [PATCHv4 7/6] conf: split out virDomainIOMMUDefCheckABIStability

2017-05-15 Thread Peter Krempa
On Fri, May 12, 2017 at 17:08:33 +0200, Ján Tomko wrote:
> ---
>  src/conf/domain_conf.c | 24 +---
>  1 file changed, 17 insertions(+), 7 deletions(-)

ACK


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

Re: [libvirt] libvirt-python RPM installation not recognized by pip

2017-05-15 Thread Daniel P. Berrange
On Mon, May 15, 2017 at 02:52:07PM +0200, Martin Kletzander wrote:
> On Mon, May 15, 2017 at 07:35:30AM -0400, Cleber Rosa wrote:
> > Hello,
> > 
> > When using the standard "requirements.txt" files for installation
> > package dependencies, I noticed that "libvirt-python" would attempt to
> > be installed by "pip" even when the equivalent RPM package is already
> > installed.
> > 
> > For instance, on a Fedora 25 system:
> > 
> >  $ rpm -q libvirt-python
> >  libvirt-python-2.2.0-1.fc25.x86_64
> >  $ python -e 'import pkg_resources;
> > pkg_resources.get_distribution("libvirt-python")'
> >  ...
> > pkg_resources.DistributionNotFound: The 'libvirt-python' distribution
> > was not found and is required by the application
> > 
> > The provider (the actual module) is actually present, but (rightfully
> > so) under the name "libvirt":
> > 
> >  $ python -c 'import pkg_resources; print
> > pkg_resources.get_provider("libvirt")'
> > 
> > 
> > At first sight, this seems to be caused by the lack of an "egg-info"
> > file, such as:
> > 
> > $PYTHON_SITE_PACKAGES/libvirt_python-2.2.0-py2.7.egg-info
> > 
> > I'm reporting a supposedly packaging issue here since the libvirt-python
> > setup.py file itself includes support for a custom rpm command.
> > 
> > Please advise if any of this is intentional, and/or whether this should
> > indeed be reported here or on downstream only.
> > 
> 
> On non-rpm system, I have that file and it works properly:
> 
>  $ ls /usr/lib64/python*/site-packages/libvirt_python-3.2.0-py*.egg-info
>  /usr/lib64/python2.7/site-packages/libvirt_python-3.2.0-py2.7.egg-info
>  /usr/lib64/python3.4/site-packages/libvirt_python-3.2.0-py3.4.egg-info
> 
>  $ pip install --user libvirt-python
>  Requirement already satisfied: libvirt-python in 
> /usr/lib64/python3.4/site-packages
> 
> Looks like this _might_ be related to the %install script in 
> libvirt-python.spec.in:
> 
>  rm -f %{buildroot}%{_libdir}/python*/site-packages/*egg-info
> 
> So it should be included instead, I guess.  Cc'ing the original author
> for confirmation that it was not intentional.

Fedora policy for a long time was that python packages *should* delete
the *egg-info metadata files, but this has apparently changed so that it
should be kept installed.

https://lists.fedoraproject.org/archives/list/de...@lists.fedoraproject.org/thread/FOG5APDRQVNXR5ZOZZSVDNZVN4WURKG4/

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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


Re: [libvirt] libvirt-python RPM installation not recognized by pip

2017-05-15 Thread Martin Kletzander

On Mon, May 15, 2017 at 07:35:30AM -0400, Cleber Rosa wrote:

Hello,

When using the standard "requirements.txt" files for installation
package dependencies, I noticed that "libvirt-python" would attempt to
be installed by "pip" even when the equivalent RPM package is already
installed.

For instance, on a Fedora 25 system:

 $ rpm -q libvirt-python
 libvirt-python-2.2.0-1.fc25.x86_64
 $ python -e 'import pkg_resources;
pkg_resources.get_distribution("libvirt-python")'
 ...
pkg_resources.DistributionNotFound: The 'libvirt-python' distribution
was not found and is required by the application

The provider (the actual module) is actually present, but (rightfully
so) under the name "libvirt":

 $ python -c 'import pkg_resources; print
pkg_resources.get_provider("libvirt")'


At first sight, this seems to be caused by the lack of an "egg-info"
file, such as:

$PYTHON_SITE_PACKAGES/libvirt_python-2.2.0-py2.7.egg-info

I'm reporting a supposedly packaging issue here since the libvirt-python
setup.py file itself includes support for a custom rpm command.

Please advise if any of this is intentional, and/or whether this should
indeed be reported here or on downstream only.



On non-rpm system, I have that file and it works properly:

 $ ls /usr/lib64/python*/site-packages/libvirt_python-3.2.0-py*.egg-info
 /usr/lib64/python2.7/site-packages/libvirt_python-3.2.0-py2.7.egg-info
 /usr/lib64/python3.4/site-packages/libvirt_python-3.2.0-py3.4.egg-info

 $ pip install --user libvirt-python
 Requirement already satisfied: libvirt-python in 
/usr/lib64/python3.4/site-packages

Looks like this _might_ be related to the %install script in 
libvirt-python.spec.in:

 rm -f %{buildroot}%{_libdir}/python*/site-packages/*egg-info

So it should be included instead, I guess.  Cc'ing the original author
for confirmation that it was not intentional.

Thanks for reporting that.


Regards,

--
Cleber Rosa
[ Sr Software Engineer - Virtualization Team - Red Hat ]
[ Avocado Test Framework - avocado-framework.github.io ]
[  7ABB 96EB 8B46 B94D 5E0F  E9BB 657E 8D33 A5F2 09F3  ]







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


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

[libvirt] [PATCH] docs: Update pointer to networking information

2017-05-15 Thread Andrea Bolognani
Commit 6fb5dd4fd804 removed docs/archnetwork.html.in, but
left behind a pointer to it in docs/formatnetwork.html.in.

Update it so that it points to the wiki, which contains
more detailed and recent information anyway.
---
 docs/formatnetwork.html.in | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
index 777c341..b410dd6 100644
--- a/docs/formatnetwork.html.in
+++ b/docs/formatnetwork.html.in
@@ -8,9 +8,9 @@
 
 
 
-  This page provides an introduction to the network XML format. For 
background
-  information on the concepts referred to here, consult the network driver architecture
-  page.
+  This page provides an introduction to the network XML format. For
+  background information on the concepts referred to here, consult the
+  https://wiki.libvirt.org/page/Networking;>relevant wiki 
page.
 
 
 Element and attribute overview
-- 
2.7.4

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


[libvirt] [PATCH v4 6/6] docs: Document the mediated devices within the nodedev driver

2017-05-15 Thread Erik Skultety
Signed-off-by: Erik Skultety 
---
 docs/drvnodedev.html.in | 168 +++-
 tools/virsh.pod |   7 +-
 2 files changed, 171 insertions(+), 4 deletions(-)

diff --git a/docs/drvnodedev.html.in b/docs/drvnodedev.html.in
index 0a3870343..26c52dd0d 100644
--- a/docs/drvnodedev.html.in
+++ b/docs/drvnodedev.html.in
@@ -9,7 +9,7 @@
   (historically also referred to as node devices) like USB, PCI, SCSI, and
   network devices. This also includes various virtualization capabilities
   which the aforementioned devices provide for utilization, for example
-  SR-IOV, NPIV, DRM, etc.
+  SR-IOV, NPIV, MDEV, DRM, etc.
 
 
 
@@ -75,6 +75,7 @@
   storage (Since 1.0.4),
   scsi_generic (Since 1.0.7),
   drm (Since 3.1.0), and
+  mdev (Since 3.4.0).
   This element can be nested in which case it further specifies a
   device's capability. Refer to specific device types to see more 
values
   for the type attribute which are exclusive.
@@ -185,5 +186,170 @@
 ...
 device
 
+MDEV capability
+
+  A PCI device capable of creating mediated devices will include a nested
+  capability mdev_types which enumerates all supported mdev
+  types on the physical device, along with the type attributes available
+  through sysfs:
+
+
+
+  type
+  
+This element describes a mediated device type which acts as an
+abstract template defining a resource allocation for instances of this
+device type. The element has one attribute id which holds
+an official vendor-supplied identifier for the type.
+Since 3.4.0
+  
+
+  name
+  
+The name element holds a vendor-supplied code name for
+the given mediated device type. This is an optional element.
+Since 3.4.0
+  
+
+  deviceAPI
+  
+The value of this element describes how an instance of the given type
+will be presented to the guest by the VFIO framework.
+Since 3.4.0
+  
+
+  availableInstances
+  
+This element reports the current state of resource allocation. In other
+words, how many instances of the given type can still be successfully
+created on the physical device.
+Since 3.4.0
+  
+
+
+
+  For a more info about mediated devices, refer to the
+  paragraph below.
+
+
+
+device
+...
+  driver
+namenvidia/name
+  /driver
+  capability type='pci'
+...
+capability type='mdev_types'
+  type id='nvidia-11'
+nameGRID M60-0B/name
+deviceAPIvfio-pci/deviceAPI
+availableInstances16/availableInstances
+  /type
+  !-- Here would come the rest of the available mdev types --
+/capability
+...
+  /capability
+/device
+
+Mediated devices (MDEVs)
+
+  Mediated devices (Since 3.2.0) are software
+  devices defining resource allocation on the backing physical device which
+  in turn allows the parent physical device's resources to be divided into
+  several mediated devices, thus sharing the physical device's performance
+  among multiple guests. Unlike SR-IOV however, where a PCIe device appears
+  as multiple separate PCIe devices on the host's PCI bus, mediated devices
+  only appear on the mdev virtual bus. Therefore, no detach/reattach
+  procedure from/to the host driver procedure is involved even though
+  mediated devices are used in a direct device assignment manner.
+
+
+
+  The following sub-elements and attributes are exposed within the
+  capability element:
+
+
+
+  type
+  
+This element describes a mediated device type which acts as an
+abstract template defining a resource allocation for instances of this
+device type. The element has one attribute id which holds
+an official vendor-supplied identifier for the type.
+Since 3.4.0
+  
+
+  iommuGroup
+  
+This element supports a single attribute number which 
holds
+the IOMMU group number the mediated device belongs to.
+Since 3.4.0
+  
+
+
+Example of a mediated device
+
+device
+  namemdev_4b20d080_1b54_4048_85b3_a6a62d165c01/name
+  
path/sys/devices/pci:00/:00:02.0/4b20d080-1b54-4048-85b3-a6a62d165c01/path
+  parentpci__06_00_0/parent
+  driver
+namevfio_mdev/name
+  /driver
+  capability type='mdev'
+type id='nvidia-11'/
+iommuGroup number='12'/
+  capability/
+device/
+
+
+  The support of mediated device's framework in libvirt's node device 
driver
+  covers the following features:
+
+
+
+  
+list available mediated devices on the host
+(Since 3.4.0)
+  
+  
+display device details
+(Since 3.4.0)
+  
+
+
+
+  Because mediated devices are instantiated from 

[libvirt] [PATCH v4 1/6] mdev: Pass a uuidstr rather than an mdev object to some util functions

2017-05-15 Thread Erik Skultety
Namely, this patch is about virMediatedDeviceGetIOMMUGroup{Dev,Num}
functions. There's no compelling reason why these functions should take
an object, on the contrary, having to create an object every time one
needs to query the IOMMU group number, discarding the object afterwards,
seems odd.

Signed-off-by: Erik Skultety 
---
 src/qemu/qemu_domain.c   |  8 +---
 src/security/security_apparmor.c | 10 +-
 src/security/security_dac.c  | 20 ++--
 src/security/security_selinux.c  | 20 ++--
 src/util/virmdev.c   | 21 +
 src/util/virmdev.h   |  4 ++--
 6 files changed, 21 insertions(+), 62 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index cc02c801e..76acb1c3f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7181,7 +7181,6 @@ qemuDomainGetHostdevPath(virDomainDefPtr def,
 virUSBDevicePtr usb = NULL;
 virSCSIDevicePtr scsi = NULL;
 virSCSIVHostDevicePtr host = NULL;
-virMediatedDevicePtr mdev = NULL;
 char *tmpPath = NULL;
 bool freeTmpPath = false;
 bool includeVFIO = false;
@@ -7282,11 +7281,7 @@ qemuDomainGetHostdevPath(virDomainDefPtr def,
 }
 
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
-if (!(mdev = virMediatedDeviceNew(mdevsrc->uuidstr,
-  mdevsrc->model)))
-goto cleanup;
-
-if (!(tmpPath = virMediatedDeviceGetIOMMUGroupDev(mdev)))
+if (!(tmpPath = 
virMediatedDeviceGetIOMMUGroupDev(mdevsrc->uuidstr)))
 goto cleanup;
 
 freeTmpPath = true;
@@ -7342,7 +7337,6 @@ qemuDomainGetHostdevPath(virDomainDefPtr def,
 virUSBDeviceFree(usb);
 virSCSIDeviceFree(scsi);
 virSCSIVHostDeviceFree(host);
-virMediatedDeviceFree(mdev);
 if (freeTmpPath)
 VIR_FREE(tmpPath);
 return ret;
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index fc5581526..62672b0af 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -905,21 +905,13 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr,
 
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
 char *vfiodev = NULL;
-virMediatedDevicePtr mdev = virMediatedDeviceNew(mdevsrc->uuidstr,
- mdevsrc->model);
 
-if (!mdev)
+if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc->uuidstr)))
 goto done;
 
-if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdev))) {
-virMediatedDeviceFree(mdev);
-goto done;
-}
-
 ret = AppArmorSetSecurityHostdevLabelHelper(vfiodev, ptr);
 
 VIR_FREE(vfiodev);
-virMediatedDeviceFree(mdev);
 break;
 }
 
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 922e48494..7dcf4c15f 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -968,21 +968,13 @@ virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr,
 
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
 char *vfiodev = NULL;
-virMediatedDevicePtr mdev = virMediatedDeviceNew(mdevsrc->uuidstr,
- mdevsrc->model);
 
-if (!mdev)
+if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc->uuidstr)))
 goto done;
 
-if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdev))) {
-virMediatedDeviceFree(mdev);
-goto done;
-}
-
 ret = virSecurityDACSetHostdevLabelHelper(vfiodev, );
 
 VIR_FREE(vfiodev);
-virMediatedDeviceFree(mdev);
 break;
 }
 
@@ -1144,21 +1136,13 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr 
mgr,
 
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
 char *vfiodev = NULL;
-virMediatedDevicePtr mdev = virMediatedDeviceNew(mdevsrc->uuidstr,
- mdevsrc->model);
 
-if (!mdev)
+if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc->uuidstr)))
 goto done;
 
-if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdev))) {
-virMediatedDeviceFree(mdev);
-goto done;
-}
-
 ret = 
virSecurityDACRestoreFileLabel(virSecurityManagerGetPrivateData(mgr),
  vfiodev);
 VIR_FREE(vfiodev);
-virMediatedDeviceFree(mdev);
 break;
 }
 
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index df7c96833..c7a2dfe98 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1843,21 +1843,13 @@ 
virSecuritySELinuxSetHostdevSubsysLabel(virSecurityManagerPtr mgr,
 
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
 char 

[libvirt] [PATCH v4 5/6] nodedev: Introduce mdev capability for mediated devices

2017-05-15 Thread Erik Skultety
Start discovering the mediated devices on the host system and format the
attributes for the mediated device into the XML. Compared to the parent
device which reports generic information about the abstract mediated
devices types, a child device only reports the type name it has been
instantiated from and the IOMMU group number, since that's device
specific compared to the rest of the info that can be gathered about
mediated devices at the moment.
This patch introduces both the formatting and parsing routines, updates
nodedev.rng schema, adding a testcase as well.

The resulting mdev child device XML:

  mdev_4b20d080_1b54_4048_85b3_a6a62d165c01
  /sys/devices/.../4b20d080-1b54-4048-85b3-a6a62d165c01
  pci__06_00_0
  
vfio_mdev
  
  


  


Signed-off-by: Erik Skultety 
---
 docs/schemas/nodedev.rng   | 17 +
 src/conf/node_device_conf.c| 41 +
 src/conf/node_device_conf.h|  8 
 src/node_device/node_device_udev.c | 43 +-
 .../mdev_3627463d_b7f0_4fea_b468_f1da537d301b.xml  |  8 
 tests/nodedevxml2xmltest.c |  1 +
 6 files changed, 117 insertions(+), 1 deletion(-)
 create mode 100644 
tests/nodedevschemadata/mdev_3627463d_b7f0_4fea_b468_f1da537d301b.xml

diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
index e0a2c5032..924f73861 100644
--- a/docs/schemas/nodedev.rng
+++ b/docs/schemas/nodedev.rng
@@ -83,6 +83,7 @@
 
 
 
+
   
 
   
@@ -580,6 +581,22 @@
 
   
 
+  
+
+  mdev
+
+
+  
+
+  
+
+
+  
+
+  
+
+  
+
   
 
   
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index f26b1ffc7..bdb6c9cf7 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -577,6 +577,10 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def)
 virBufferEscapeString(, "%s\n", 
virNodeDevDRMTypeToString(data->drm.type));
 break;
 case VIR_NODE_DEV_CAP_MDEV:
+virBufferEscapeString(, "\n", data->mdev.type);
+virBufferAsprintf(, "\n",
+  data->mdev.iommuGroupNumber);
+break;
 case VIR_NODE_DEV_CAP_MDEV_TYPES:
 case VIR_NODE_DEV_CAP_FC_HOST:
 case VIR_NODE_DEV_CAP_VPORTS:
@@ -1643,6 +1647,39 @@ virNodeDevCapSystemParseXML(xmlXPathContextPtr ctxt,
 }
 
 
+static int
+virNodeDevCapMdevParseXML(xmlXPathContextPtr ctxt,
+  virNodeDeviceDefPtr def,
+  xmlNodePtr node,
+  virNodeDevCapMdevPtr mdev)
+{
+xmlNodePtr orignode;
+int ret = -1;
+
+orignode = ctxt->node;
+ctxt->node = node;
+
+if (!(mdev->type = virXPathString("string(./type[1]/@id)", ctxt))) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("missing type id attribute for '%s'"), def->name);
+goto out;
+}
+
+if (virNodeDevCapsDefParseULong("number(./iommuGroup[1]/@number)", ctxt,
+>iommuGroupNumber, def,
+_("missing iommuGroup number atribute for "
+  "'%s'"),
+_("invalid iommuGroup number attribute for 
"
+  "'%s'")) < 0)
+goto out;
+
+ret = 0;
+ out:
+ctxt->node = orignode;
+return ret;
+}
+
+
 static virNodeDevCapsDefPtr
 virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt,
   virNodeDeviceDefPtr def,
@@ -1711,6 +1748,8 @@ virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt,
 ret = virNodeDevCapDRMParseXML(ctxt, def, node, >data.drm);
 break;
 case VIR_NODE_DEV_CAP_MDEV:
+ret = virNodeDevCapMdevParseXML(ctxt, def, node, >data.mdev);
+break;
 case VIR_NODE_DEV_CAP_MDEV_TYPES:
 case VIR_NODE_DEV_CAP_FC_HOST:
 case VIR_NODE_DEV_CAP_VPORTS:
@@ -2034,6 +2073,8 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
 VIR_FREE(data->sg.path);
 break;
 case VIR_NODE_DEV_CAP_MDEV:
+VIR_FREE(data->mdev.type);
+break;
 case VIR_NODE_DEV_CAP_MDEV_TYPES:
 case VIR_NODE_DEV_CAP_DRM:
 case VIR_NODE_DEV_CAP_FC_HOST:
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index 629f732e6..cea9247d2 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -143,6 +143,13 @@ struct _virNodeDevCapMdevType {
 unsigned int available_instances;
 };
 
+typedef struct _virNodeDevCapMdev virNodeDevCapMdev;
+typedef virNodeDevCapMdev *virNodeDevCapMdevPtr;
+struct _virNodeDevCapMdev {
+char *type;
+unsigned int iommuGroupNumber;
+};
+
 typedef struct _virNodeDevCapPCIDev virNodeDevCapPCIDev;
 typedef virNodeDevCapPCIDev 

[libvirt] [PATCH v4 3/6] nodedev: Introduce new mdev_types and mdev nodedev capabilities

2017-05-15 Thread Erik Skultety
The reason for introducing two capabilities, one for the device itself
(cap 'mdev') and one for the parent device listing the available types
('mdev_types'), is that we should be able to do
'virsh nodedev-list --cap' not only for existing mdev devices but also
for devices that support creation of mdev devices, since one day libvirt
might be actually able to create the mdev devices in an automated way
(just like we do for NPIV/vHBA).

Signed-off-by: Erik Skultety 
---
 include/libvirt/libvirt-nodedev.h|  2 ++
 src/conf/node_device_conf.c  | 10 +-
 src/conf/node_device_conf.h  |  6 +-
 src/conf/virnodedeviceobj.c  |  4 +++-
 src/libvirt-nodedev.c|  2 ++
 src/node_device/node_device_driver.c |  2 ++
 src/node_device/node_device_udev.c   |  3 +++
 tools/virsh-nodedev.c|  6 ++
 8 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/include/libvirt/libvirt-nodedev.h 
b/include/libvirt/libvirt-nodedev.h
index 85003903d..1e3043787 100644
--- a/include/libvirt/libvirt-nodedev.h
+++ b/include/libvirt/libvirt-nodedev.h
@@ -79,6 +79,8 @@ typedef enum {
 VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS= 1 << 10, /* Capable of 
vport */
 VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_GENERIC  = 1 << 11, /* Capable of 
scsi_generic */
 VIR_CONNECT_LIST_NODE_DEVICES_CAP_DRM   = 1 << 12, /* DRM device */
+VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV_TYPES= 1 << 13, /* Capable of 
mediated devices */
+VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV  = 1 << 14, /* Mediated 
device */
 } virConnectListAllNodeDeviceFlags;
 
 int virConnectListAllNodeDevices (virConnectPtr conn,
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 7aab2e03c..40d71f277 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -60,7 +60,9 @@ VIR_ENUM_IMPL(virNodeDevCap, VIR_NODE_DEV_CAP_LAST,
   "fc_host",
   "vports",
   "scsi_generic",
-  "drm")
+  "drm",
+  "mdev_types",
+  "mdev")
 
 VIR_ENUM_IMPL(virNodeDevNetCap, VIR_NODE_DEV_CAP_NET_LAST,
   "80203",
@@ -540,6 +542,8 @@ virNodeDeviceDefFormat(const virNodeDeviceDef *def)
 case VIR_NODE_DEV_CAP_DRM:
 virBufferEscapeString(, "%s\n", 
virNodeDevDRMTypeToString(data->drm.type));
 break;
+case VIR_NODE_DEV_CAP_MDEV:
+case VIR_NODE_DEV_CAP_MDEV_TYPES:
 case VIR_NODE_DEV_CAP_FC_HOST:
 case VIR_NODE_DEV_CAP_VPORTS:
 case VIR_NODE_DEV_CAP_LAST:
@@ -1612,6 +1616,8 @@ virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt,
 case VIR_NODE_DEV_CAP_DRM:
 ret = virNodeDevCapDRMParseXML(ctxt, def, node, >data.drm);
 break;
+case VIR_NODE_DEV_CAP_MDEV:
+case VIR_NODE_DEV_CAP_MDEV_TYPES:
 case VIR_NODE_DEV_CAP_FC_HOST:
 case VIR_NODE_DEV_CAP_VPORTS:
 case VIR_NODE_DEV_CAP_SCSI_GENERIC:
@@ -1930,6 +1936,8 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
 case VIR_NODE_DEV_CAP_SCSI_GENERIC:
 VIR_FREE(data->sg.path);
 break;
+case VIR_NODE_DEV_CAP_MDEV:
+case VIR_NODE_DEV_CAP_MDEV_TYPES:
 case VIR_NODE_DEV_CAP_DRM:
 case VIR_NODE_DEV_CAP_FC_HOST:
 case VIR_NODE_DEV_CAP_VPORTS:
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index a5d5cdd2a..273d49f76 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -64,6 +64,8 @@ typedef enum {
 VIR_NODE_DEV_CAP_VPORTS,   /* HBA which is capable of vports */
 VIR_NODE_DEV_CAP_SCSI_GENERIC,  /* SCSI generic device */
 VIR_NODE_DEV_CAP_DRM,   /* DRM device */
+VIR_NODE_DEV_CAP_MDEV_TYPES,  /* Device capable of mediated devices */
+VIR_NODE_DEV_CAP_MDEV,/* Mediated device */
 
 VIR_NODE_DEV_CAP_LAST
 } virNodeDevCapType;
@@ -351,7 +353,9 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps);
  VIR_CONNECT_LIST_NODE_DEVICES_CAP_FC_HOST   | \
  VIR_CONNECT_LIST_NODE_DEVICES_CAP_VPORTS| \
  VIR_CONNECT_LIST_NODE_DEVICES_CAP_SCSI_GENERIC  | \
- VIR_CONNECT_LIST_NODE_DEVICES_CAP_DRM)
+ VIR_CONNECT_LIST_NODE_DEVICES_CAP_DRM   | \
+ VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV_TYPES| \
+ VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV)
 
 char *
 virNodeDeviceGetParentName(virConnectPtr conn,
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index 4f47b4e41..181d2efe1 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -550,7 +550,9 @@ virNodeDeviceMatch(virNodeDeviceObjPtr devobj,
   MATCH(FC_HOST)   ||
   MATCH(VPORTS)||
   MATCH(SCSI_GENERIC)  ||
-  MATCH(DRM)))
+  MATCH(DRM)   ||
+  

[libvirt] [PATCH v4 4/6] nodedev: Introduce the mdev capability to a PCI parent device

2017-05-15 Thread Erik Skultety
The parent device needs to report the generic stuff about the supported
mediated devices types, like device API, available instances, type name,
etc. Therefore this patch introduces a new nested capability element of
type 'mdev_types' with the resulting XML of the following format:


  ...
  
...

  
optional_vendor_supplied_codename
vfio-pci
NUM
  
...
  
...
  

  
  ...


Signed-off-by: Erik Skultety 
---
 docs/schemas/nodedev.rng   |  26 +
 src/conf/node_device_conf.c|  97 +
 src/conf/node_device_conf.h|  15 +++
 src/conf/virnodedeviceobj.c|   7 ++
 src/libvirt_private.syms   |   1 +
 src/node_device/node_device_udev.c | 119 +
 .../pci__02_10_7_mdev_types.xml|  32 ++
 tests/nodedevxml2xmltest.c |   1 +
 8 files changed, 298 insertions(+)
 create mode 100644 tests/nodedevschemadata/pci__02_10_7_mdev_types.xml

diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng
index 0f90a73c8..e0a2c5032 100644
--- a/docs/schemas/nodedev.rng
+++ b/docs/schemas/nodedev.rng
@@ -205,6 +205,32 @@
 
 
 
+  
+
+  mdev_types
+
+
+  
+
+  
+
+
+  
+
+
+  
+vfio-pci
+  
+
+
+  
+
+  
+
+  
+   
+
+
   
 
   
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 40d71f277..f26b1ffc7 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -89,6 +89,19 @@ virNodeDevCapsDefParseString(const char *xpath,
 
 
 void
+virNodeDevCapMdevTypeFree(virNodeDevCapMdevTypePtr type)
+{
+if (!type)
+return;
+
+VIR_FREE(type->id);
+VIR_FREE(type->name);
+VIR_FREE(type->device_api);
+VIR_FREE(type);
+}
+
+
+void
 virNodeDeviceDefFree(virNodeDeviceDefPtr def)
 {
 virNodeDevCapsDefPtr caps;
@@ -265,6 +278,27 @@ virNodeDeviceCapPCIDefFormat(virBufferPtr buf,
 virBufferAsprintf(buf, "\n",
   virPCIHeaderTypeToString(data->pci_dev.hdrType));
 }
+if (data->pci_dev.flags & VIR_NODE_DEV_CAP_FLAG_PCI_MDEV) {
+virBufferAddLit(buf, "\n");
+virBufferAdjustIndent(buf, 2);
+for (i = 0; i < data->pci_dev.nmdev_types; i++) {
+virNodeDevCapMdevTypePtr type = data->pci_dev.mdev_types[i];
+virBufferEscapeString(buf, "\n", type->id);
+virBufferAdjustIndent(buf, 2);
+if (type->name)
+virBufferEscapeString(buf, "%s\n",
+  type->name);
+virBufferEscapeString(buf, "%s\n",
+  type->device_api);
+virBufferAsprintf(buf,
+  "%u\n",
+  type->available_instances);
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, "\n");
+}
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, "\n");
+}
 if (data->pci_dev.nIommuGroupDevices) {
 virBufferAsprintf(buf, "\n",
   data->pci_dev.iommuGroupNumber);
@@ -1365,6 +1399,63 @@ virNodeDevPCICapSRIOVVirtualParseXML(xmlXPathContextPtr 
ctxt,
 
 
 static int
+virNodeDevPCICapMdevTypesParseXML(xmlXPathContextPtr ctxt,
+  virNodeDevCapPCIDevPtr pci_dev)
+{
+int ret = -1;
+xmlNodePtr orignode = NULL;
+xmlNodePtr *nodes = NULL;
+int nmdev_types = virXPathNodeSet("./type", ctxt, );
+virNodeDevCapMdevTypePtr type = NULL;
+size_t i;
+
+orignode = ctxt->node;
+for (i = 0; i < nmdev_types; i++) {
+ctxt->node = nodes[i];
+
+if (VIR_ALLOC(type) < 0)
+goto cleanup;
+
+if (!(type->id = virXPathString("string(./@id[1])", ctxt))) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("missing 'id' attribute for mediated device's "
+ " element"));
+goto cleanup;
+}
+
+if (!(type->device_api = virXPathString("string(./deviceAPI[1])", 
ctxt))) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("missing device API for mediated device type 
'%s'"),
+   type->id);
+goto cleanup;
+}
+
+if (virXPathUInt("number(./availableInstances)", ctxt,
+ >available_instances) < 0) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("missing number of available instances for "
+ "mediated device type '%s'"),
+

[libvirt] [PATCH v4 0/6] Add mdev reporting capability to the nodedev driver

2017-05-15 Thread Erik Skultety
since v1:
- dropped the  element from the parent device nested capability
- added missing RNG schema and tests
- updated the documentation to describe the MDEV elements in both the parent
and the child

since v2:
- I further split our PCI sub-capability parser into more blocks as suggested
- instead of one capability 'mdev' for both mdev device and physical parent I
introduced 2, so we can do virsh nodedev-list --cap 'mdev_types' | 'mdev' to
see either parent devices or the mediated devices themselves
- other minor adjustments pointed out during review.

since v3:
- fixed nits
- updated virsh man page to include mdev and mdev_types within the list of
supported capabilities

Erik Skultety (6):
  mdev: Pass a uuidstr rather than an mdev object to some util functions
  nodedev: conf: Split PCI sub-capability parsing to separate methods
  nodedev: Introduce new mdev_types and mdev nodedev capabilities
  nodedev: Introduce the mdev capability to a PCI parent device
  nodedev: Introduce mdev capability for mediated devices
  docs: Document the mediated devices within the nodedev driver

 docs/drvnodedev.html.in| 168 +++-
 docs/schemas/nodedev.rng   |  43 +++
 include/libvirt/libvirt-nodedev.h  |   2 +
 src/conf/node_device_conf.c| 290 -
 src/conf/node_device_conf.h|  29 ++-
 src/conf/virnodedeviceobj.c|  11 +-
 src/libvirt-nodedev.c  |   2 +
 src/libvirt_private.syms   |   1 +
 src/node_device/node_device_driver.c   |   2 +
 src/node_device/node_device_udev.c | 165 +++-
 src/qemu/qemu_domain.c |   8 +-
 src/security/security_apparmor.c   |  10 +-
 src/security/security_dac.c|  20 +-
 src/security/security_selinux.c|  20 +-
 src/util/virmdev.c |  21 +-
 src/util/virmdev.h |   4 +-
 .../mdev_3627463d_b7f0_4fea_b468_f1da537d301b.xml  |   8 +
 .../pci__02_10_7_mdev_types.xml|  32 +++
 tests/nodedevxml2xmltest.c |   2 +
 tools/virsh-nodedev.c  |   6 +
 tools/virsh.pod|   7 +-
 21 files changed, 722 insertions(+), 129 deletions(-)
 create mode 100644 
tests/nodedevschemadata/mdev_3627463d_b7f0_4fea_b468_f1da537d301b.xml
 create mode 100644 tests/nodedevschemadata/pci__02_10_7_mdev_types.xml

--
2.13.0

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


[libvirt] [PATCH v4 2/6] nodedev: conf: Split PCI sub-capability parsing to separate methods

2017-05-15 Thread Erik Skultety
Since there's at least SRIOV and MDEV sub-capabilities to be parsed,
let's make the code more readable by splitting it to several logical
blocks.

Signed-off-by: Erik Skultety 
---
 src/conf/node_device_conf.c | 142 ++--
 1 file changed, 83 insertions(+), 59 deletions(-)

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 85cfd8396..7aab2e03c 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -1286,76 +1286,102 @@ virPCIEDeviceInfoParseXML(xmlXPathContextPtr ctxt,
 
 
 static int
+virNodeDevPCICapSRIOVPhysicalParseXML(xmlXPathContextPtr ctxt,
+  virNodeDevCapPCIDevPtr pci_dev)
+{
+xmlNodePtr address = virXPathNode("./address[1]", ctxt);
+
+if (VIR_ALLOC(pci_dev->physical_function) < 0)
+return -1;
+
+if (!address) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("Missing address in 'phys_function' capability"));
+return -1;
+}
+
+if (virPCIDeviceAddressParseXML(address,
+pci_dev->physical_function) < 0)
+return -1;
+
+pci_dev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
+
+return 0;
+}
+
+
+static int
+virNodeDevPCICapSRIOVVirtualParseXML(xmlXPathContextPtr ctxt,
+ virNodeDevCapPCIDevPtr pci_dev)
+{
+int ret = -1;
+xmlNodePtr *addresses = NULL;
+int naddresses = virXPathNodeSet("./address", ctxt, );
+char *maxFuncsStr = virXPathString("string(./@maxCount)", ctxt);
+size_t i;
+
+if (naddresses < 0)
+goto cleanup;
+
+if (maxFuncsStr &&
+virStrToLong_uip(maxFuncsStr, NULL, 10,
+ _dev->max_virtual_functions) < 0) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("Malformed 'maxCount' parameter"));
+goto cleanup;
+}
+
+if (VIR_ALLOC_N(pci_dev->virtual_functions, naddresses) < 0)
+goto cleanup;
+
+for (i = 0; i < naddresses; i++) {
+virPCIDeviceAddressPtr addr = NULL;
+
+if (VIR_ALLOC(addr) < 0)
+goto cleanup;
+
+if (virPCIDeviceAddressParseXML(addresses[i], addr) < 0) {
+VIR_FREE(addr);
+goto cleanup;
+}
+
+if (VIR_APPEND_ELEMENT(pci_dev->virtual_functions,
+   pci_dev->num_virtual_functions,
+   addr) < 0)
+goto cleanup;
+}
+
+pci_dev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
+ret = 0;
+ cleanup:
+VIR_FREE(addresses);
+VIR_FREE(maxFuncsStr);
+return ret;
+}
+
+
+static int
 virNodeDevPCICapabilityParseXML(xmlXPathContextPtr ctxt,
 xmlNodePtr node,
 virNodeDevCapPCIDevPtr pci_dev)
 {
-char *maxFuncsStr = virXMLPropString(node, "maxCount");
 char *type = virXMLPropString(node, "type");
-xmlNodePtr *addresses = NULL;
 xmlNodePtr orignode = ctxt->node;
 int ret = -1;
-size_t i = 0;
 
 ctxt->node = node;
 
 if (!type) {
 virReportError(VIR_ERR_XML_ERROR, "%s", _("Missing capability type"));
-goto out;
+goto cleanup;
 }
 
-if (STREQ(type, "phys_function")) {
-xmlNodePtr address = virXPathNode("./address[1]", ctxt);
-
-if (VIR_ALLOC(pci_dev->physical_function) < 0)
-goto out;
-
-if (!address) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
-   _("Missing address in 'phys_function' capability"));
-goto out;
-}
-
-if (virPCIDeviceAddressParseXML(address,
-pci_dev->physical_function) < 0)
-goto out;
-
-pci_dev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
-} else if (STREQ(type, "virt_functions")) {
-int naddresses;
-
-if ((naddresses = virXPathNodeSet("./address", ctxt, )) < 0)
-goto out;
-
-if (maxFuncsStr &&
-virStrToLong_uip(maxFuncsStr, NULL, 10,
- _dev->max_virtual_functions) < 0) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
-   _("Malformed 'maxCount' parameter"));
-goto out;
-}
-
-if (VIR_ALLOC_N(pci_dev->virtual_functions, naddresses) < 0)
-goto out;
-
-for (i = 0; i < naddresses; i++) {
-virPCIDeviceAddressPtr addr = NULL;
-
-if (VIR_ALLOC(addr) < 0)
-goto out;
-
-if (virPCIDeviceAddressParseXML(addresses[i], addr) < 0) {
-VIR_FREE(addr);
-goto out;
-}
-
-if (VIR_APPEND_ELEMENT(pci_dev->virtual_functions,
-   pci_dev->num_virtual_functions,
-   addr) < 0)
-goto out;
-}

[libvirt] libvirt-python RPM installation not recognized by pip

2017-05-15 Thread Cleber Rosa
Hello,

When using the standard "requirements.txt" files for installation
package dependencies, I noticed that "libvirt-python" would attempt to
be installed by "pip" even when the equivalent RPM package is already
installed.

For instance, on a Fedora 25 system:

  $ rpm -q libvirt-python
  libvirt-python-2.2.0-1.fc25.x86_64
  $ python -e 'import pkg_resources;
pkg_resources.get_distribution("libvirt-python")'
  ...
 pkg_resources.DistributionNotFound: The 'libvirt-python' distribution
was not found and is required by the application

The provider (the actual module) is actually present, but (rightfully
so) under the name "libvirt":

  $ python -c 'import pkg_resources; print
pkg_resources.get_provider("libvirt")'
 

At first sight, this seems to be caused by the lack of an "egg-info"
file, such as:

$PYTHON_SITE_PACKAGES/libvirt_python-2.2.0-py2.7.egg-info

I'm reporting a supposedly packaging issue here since the libvirt-python
setup.py file itself includes support for a custom rpm command.

Please advise if any of this is intentional, and/or whether this should
indeed be reported here or on downstream only.

Regards,

-- 
Cleber Rosa
[ Sr Software Engineer - Virtualization Team - Red Hat ]
[ Avocado Test Framework - avocado-framework.github.io ]
[  7ABB 96EB 8B46 B94D 5E0F  E9BB 657E 8D33 A5F2 09F3  ]



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

Re: [libvirt] [PATCH v2] storage: use 0711 as the default perms for dirs

2017-05-15 Thread Christian Ehrhardt
On Mon, May 15, 2017 at 1:10 PM, Daniel P. Berrange 
wrote:

> BTW, for libvir-list we recommend to send v2/v3/etc followup patches as
> top level threads, not in-reply-to the previous versions.
>

I need a mapper which project prefers what :-), no really - thank you a lot!
Since we are about to submit a bigger pile of apparmor changes that hint
might certainly be handy the next days/weeks.


-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCHv4 1/6] conf: add to

2017-05-15 Thread Andrea Bolognani
On Fri, 2017-05-12 at 16:42 +0200, Ján Tomko wrote:
> Re: x86-only: would squashing this in do?

Yup, looks reasonable enough :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH v2] storage: use 0711 as the default perms for dirs

2017-05-15 Thread Daniel P. Berrange
On Mon, May 15, 2017 at 01:05:31PM +0200, Christian Ehrhardt wrote:
> From: Serge Hallyn 
> 
> There should be no need to make dir based pools world/group readable.
> So use 0711, not 0755, as the default perms for storage dirs.
> 
> Updates in v2:
>  - adapt commit wording to mention dropping group readable as well
> 
> Signed-off-by: Christian Ehrhardt 
> ---
>  docs/formatstorage.html.in | 2 +-
>  src/storage/storage_util.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrange 

Will push to git shortly.

BTW, for libvir-list we recommend to send v2/v3/etc followup patches as
top level threads, not in-reply-to the previous versions.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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


[libvirt] [PATCH v2] storage: use 0711 as the default perms for dirs

2017-05-15 Thread Christian Ehrhardt
From: Serge Hallyn 

There should be no need to make dir based pools world/group readable.
So use 0711, not 0755, as the default perms for storage dirs.

Updates in v2:
 - adapt commit wording to mention dropping group readable as well

Signed-off-by: Christian Ehrhardt 
---
 docs/formatstorage.html.in | 2 +-
 src/storage/storage_util.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index 225e190..4946ddf 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -444,7 +444,7 @@
 namespace. It provides information about the permissions to use for the
 final directory when the pool is built. There are 4 child elements.
 The mode element contains the octal permission set.
-The mode defaults to 0755 when not provided.
+The mode defaults to 0711 when not provided.
 The owner element contains the numeric user ID.
 The group element contains the numeric group ID.
 If owner or group aren't specified when
diff --git a/src/storage/storage_util.h b/src/storage/storage_util.h
index a05c35d..6f2a1b1 100644
--- a/src/storage/storage_util.h
+++ b/src/storage/storage_util.h
@@ -138,7 +138,7 @@ int virStorageBackendVolOpen(const char *path, struct stat 
*sb,
 ATTRIBUTE_RETURN_CHECK
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
-# define VIR_STORAGE_DEFAULT_POOL_PERM_MODE 0755
+# define VIR_STORAGE_DEFAULT_POOL_PERM_MODE 0711
 # define VIR_STORAGE_DEFAULT_VOL_PERM_MODE  0600
 
 int virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol,
-- 
2.7.4

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


Re: [libvirt] [PATCH] storage: use 0711 as the default perms for dirs

2017-05-15 Thread Christian Ehrhardt
On Fri, May 12, 2017 at 12:36 AM, John Ferlan  wrote:

> Also your commit message notes "world readable", but by going from 755
> to 711, you're also changing to "group readable" too ;-)
>

Good catch John,
the other feedback seems good, so for now I'm just rewording in regard to
this and resubmit to the thread.


-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] storage: use 0711 as the default perms for dirs

2017-05-15 Thread Christian Ehrhardt
On Mon, May 15, 2017 at 10:27 AM, Daniel P. Berrange 
wrote:

> > Kinda surprised this didn't generate some immediate discussion...  I
> > would also think that if you had a desire to change defaults you'd also
> > have a libvirt.spec.in adjustment...
>
> Actually no it doesn't - the spec file is already marking
> /var/lib/libvirt/images as 0711.


As reference that is the current spec content:
 libvirt.spec.in:1745:%dir %attr(0711, root, root)
%{_localstatedir}/lib/libvirt/images/


> > Still 0755 or umask(022) seem to be fairly prevalent setting and having
> > the  for the XML to be able to override a default certainly gives
> > credence to arguments in either direction whether or not to change the
> > defaults.
> >
> > It's been a long while since I considered system/directory/file security
> > things, but I have this faint recollection of some strange issue when
> > not having world or group "executable" as a default.
>
> The fact that RPM spec ships with 0711 show that it works ok. So I
> think this change is reasonable.


Interesting, I didn't check the RPM spec - thanks Daniel to point this out.
It is 711 on Ubuntu as well for quite some time now.
Both together make this even less likely to have hidden drawbacks.


-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 5/5] news: Update for GIC version on TCG changes

2017-05-15 Thread Peter Krempa
On Fri, May 12, 2017 at 16:14:47 +0200, Andrea Bolognani wrote:
> Signed-off-by: Andrea Bolognani 
> ---
>  docs/news.xml | 11 +++
>  1 file changed, 11 insertions(+)

ACK


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

Re: [libvirt] [PATCH] storage: use 0711 as the default perms for dirs

2017-05-15 Thread Martin Kletzander

On Mon, May 15, 2017 at 09:27:38AM +0100, Daniel P. Berrange wrote:

On Thu, May 11, 2017 at 06:36:22PM -0400, John Ferlan wrote:



On 05/11/2017 04:31 AM, Christian Ehrhardt wrote:
> From: Serge Hallyn 
>
> There should be no need to make dir based pools world readable.
> So use 0711, not 0755, as the default perms for storage dirs.
>
> Signed-off-by: Christian Ehrhardt 
> ---
>  docs/formatstorage.html.in | 2 +-
>  src/storage/storage_util.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>

Kinda surprised this didn't generate some immediate discussion...  I
would also think that if you had a desire to change defaults you'd also
have a libvirt.spec.in adjustment...


Actually no it doesn't - the spec file is already marking
/var/lib/libvirt/images as 0711.


Still 0755 or umask(022) seem to be fairly prevalent setting and having
the  for the XML to be able to override a default certainly gives
credence to arguments in either direction whether or not to change the
defaults.

It's been a long while since I considered system/directory/file security
things, but I have this faint recollection of some strange issue when
not having world or group "executable" as a default.


The fact that RPM spec ships with 0711 show that it works ok. So I
think this change is reasonable.



Same here.  I'm not sure, but I think even SELinux policy defaulted to
that.  Anyway, ACK to this one, I'll push this in a while.

While we're on this, is there some global config for the group in all
these permissions?  I would love to add a user to one group and make all
libvirt-related readable for that user with that one simple addition.



Regards,
Daniel
--
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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


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

Re: [libvirt] [PATCH 4/5] gic: Remove VIR_GIC_VERSION_DEFAULT

2017-05-15 Thread Peter Krempa
On Fri, May 12, 2017 at 16:14:46 +0200, Andrea Bolognani wrote:
> The QEMU default is GICv2, and some of the code in libvirt
> relies on the exact value. Stop pretending that's not the
> case and use GICv2 explicitly where needed.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  src/qemu/qemu_command.c | 6 +++---
>  src/qemu/qemu_domain.c  | 7 +++
>  src/util/virgic.h   | 3 ---
>  3 files changed, 6 insertions(+), 10 deletions(-)

ACK


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

Re: [libvirt] [PATCH 3/5] qemu: Use GICv2 for aarch64/virt TCG guests

2017-05-15 Thread Peter Krempa
On Fri, May 12, 2017 at 16:14:45 +0200, Andrea Bolognani wrote:
> There are currently some limitations in the emulated GICv3
> that make it unsuitable as a default. Use GICv2 instead.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1450433
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  src/qemu/qemu_domain.c | 41 
> +++---
>  .../qemuxml2argv-aarch64-gic-none-tcg.args |  2 +-
>  .../qemuxml2xmlout-aarch64-gic-none-tcg.xml|  2 +-
>  3 files changed, 31 insertions(+), 14 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index cc02c80..31ed391 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -2559,17 +2559,31 @@ qemuDomainDefEnableDefaultFeatures(virDomainDefPtr 
> def,
>  if (def->features[VIR_DOMAIN_FEATURE_GIC] == VIR_TRISTATE_SWITCH_ABSENT 
> &&
>  qemuDomainIsVirt(def)) {
>  
> -VIR_DEBUG("Looking for usable GIC version in domain capabilities");
> -for (version = VIR_GIC_VERSION_LAST - 1;
> - version > VIR_GIC_VERSION_NONE;
> - version--) {
> -if (virQEMUCapsSupportsGICVersion(qemuCaps,
> -  def->virtType,
> -  version)) {
> -VIR_DEBUG("Using GIC version %s",
> -  virGICVersionTypeToString(version));
> -def->gic_version = version;
> -break;
> +/* We want to use the highest available GIC version for guests;
> + * however, the emulated GICv3 is currently lacking a MSI controller,
> + * making it unsuitable for the pure PCIe topology we aim for.
> + *
> + * For that reason, we skip this step entirely for TCG guests,
> + * and rely on the code below to pick the default version, GICv2,
> + * which supports all the features we need.
> + *
> + * We'll want to revisit this once MSI support for GICv3 has been
> + * implemented in QEMU.
> + *
> + * See https://bugzilla.redhat.com/show_bug.cgi?id=1414081 */
> +if (def->virtType == VIR_DOMAIN_VIRT_KVM) {

Currently it does not matter that much, since there are only two
versions but this looks very non-future-proof to me.

When qemu adds the feature you'll need to add a capability, where you
also enable the code below for TCG guests.

If there will be another version or something the condition will need to
be altered.

I'd rather see that v3 is specifically disqualified for TCG guests
(which will be later relaxed using the capability.). That way you'll
still run the detection process.

> +VIR_DEBUG("Looking for usable GIC version in domain 
> capabilities");
> +for (version = VIR_GIC_VERSION_LAST - 1;
> + version > VIR_GIC_VERSION_NONE;
> + version--) {
> +if (virQEMUCapsSupportsGICVersion(qemuCaps,
> +  def->virtType,
> +  version)) {
> +VIR_DEBUG("Using GIC version %s",
> +  virGICVersionTypeToString(version));
> +def->gic_version = version;
> +break;
> +}
>  }
>  }


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

Re: [libvirt] [PATCH 2/5] tests: Check default GIC version for aarch64/virt TCG guests

2017-05-15 Thread Peter Krempa
On Fri, May 12, 2017 at 16:14:44 +0200, Andrea Bolognani wrote:
> Signed-off-by: Andrea Bolognani 
> ---
>  .../qemuxml2argv-aarch64-gic-none-tcg.args | 19 
>  .../qemuxml2argv-aarch64-gic-none-tcg.xml  | 17 +++
>  tests/qemuxml2argvtest.c   |  3 +++
>  .../qemuxml2xmlout-aarch64-gic-none-tcg.xml| 25 
> ++
>  tests/qemuxml2xmltest.c|  1 +
>  5 files changed, 65 insertions(+)
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.args
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-aarch64-gic-none-tcg.xml
>  create mode 100644 
> tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-gic-none-tcg.xml

ACK


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

Re: [libvirt] [PATCH 1/5] qemu: Use qemuDomainMachineIsVirt() more

2017-05-15 Thread Peter Krempa
On Fri, May 12, 2017 at 16:14:43 +0200, Andrea Bolognani wrote:
> Signed-off-by: Andrea Bolognani 
> ---
>  src/qemu/qemu_capabilities.c | 7 +--
>  1 file changed, 1 insertion(+), 6 deletions(-)

ACK


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

Re: [libvirt] [PATCH v2 3/3] qemu: improve detection of UNIX path generated by libvirt

2017-05-15 Thread Martin Kletzander

On Fri, May 12, 2017 at 04:45:11PM +0200, Pavel Hrdina wrote:

On Fri, May 12, 2017 at 04:26:35PM +0200, Martin Kletzander wrote:

On Fri, May 12, 2017 at 02:57:56PM +0200, Pavel Hrdina wrote:
>Currently we consider all UNIX paths with specific prefix as generated
>by libvirt, but that's a wrong assumption.  Let's make the detection
>better by actually checking whether the whole path matches one of the
>paths that we generate or generated in the past.
>
>The UNIX path isn't stored in config XML since libvirt-1.3.0.
>

1.3.1, I believe.

>Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1446980
>
>Signed-off-by: Pavel Hrdina 
>---
>
>changes in v2:
>- dropped the magic to split the path into 3 parts and use only one
>  regexp to match the path
>
> src/qemu/qemu_domain.c | 51 ++
> .../qemuxml2argv-channel-unix-gen-path1.xml| 17 
> .../qemuxml2argv-channel-unix-gen-path2.xml| 17 
> .../qemuxml2argv-channel-unix-gen-path3.xml| 17 
> .../qemuxml2argv-channel-unix-user-path.xml| 17 
> .../qemuxml2xmlout-channel-unix-gen-path1.xml  | 32 ++
> .../qemuxml2xmlout-channel-unix-gen-path2.xml  | 32 ++
> .../qemuxml2xmlout-channel-unix-gen-path3.xml  | 32 ++
> .../qemuxml2xmlout-channel-unix-user-path.xml  | 33 ++
> tests/qemuxml2xmltest.c|  5 +++
> 10 files changed, 244 insertions(+), 9 deletions(-)
> create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path1.xml
> create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path2.xml
> create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-channel-unix-gen-path3.xml
> create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-channel-unix-user-path.xml
> create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path1.xml
> create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path2.xml
> create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-gen-path3.xml
> create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-channel-unix-user-path.xml
>

Just have one file that tests it all.

>diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>index cc02c801e1..00e37d3428 100644
>--- a/src/qemu/qemu_domain.c
>+++ b/src/qemu/qemu_domain.c
>@@ -3154,24 +3154,57 @@ qemuDomainDefaultNetModel(const virDomainDef *def,
>
>
> /*
>- * Clear auto generated unix socket path, i.e., the one which starts with our
>- * channel directory.
>+ * Clear auto generated unix socket paths:
>+ *
>+ * libvirt 1.2.18 and older:
>+ * {cfg->channelTargetDir}/{dom-name}.{target-name}
>+ *
>+ * libvirt 1.2.19 - 1.3.2:
>+ * {cfg->channelTargetDir}/domain-{dom-name}/{target-name}
>+ *
>+ * libvirt 1.3.3 and newer:
>+ * {cfg->channelTargetDir}/domain-{dom-id}-{short-dom-name}/{target-name}
>+ *
>+ * The unix socket path was stored in config XML until libvirt 1.3.0.
>+ * If someone specifies the same path as we generate, they shouldn't do it.
>+ *
>+ * This function clears the path for migration as well, so we need to clear
>+ * the path event if we are not storing it in the XML.
>  */
>-static void
>+static int

This ^^ is not reflected anywhere.  It's a pity that such function (that
just conditionally frees something) can fail.


I've somehow lost the change to the callers to handle the failure, sigh.



> qemuDomainChrDefDropDefaultPath(virDomainChrDefPtr chr,
> virQEMUDriverPtr driver)
> {
> virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>+virBuffer buf = VIR_BUFFER_INITIALIZER;
>+char *regexp = NULL;
>+int ret = -1;
>
>-if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
>-chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
>-chr->source->type == VIR_DOMAIN_CHR_TYPE_UNIX &&
>-chr->source->data.nix.path &&
>-STRPREFIX(chr->source->data.nix.path, cfg->channelTargetDir)) {
>+if (chr->deviceType != VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL ||
>+chr->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO ||
>+chr->source->type != VIR_DOMAIN_CHR_TYPE_UNIX ||
>+!chr->source->data.nix.path) {

This would be more readable if you postponed the initialization of @cfg
and just return 0 from this.  Optionally break this into multiple
conditions.

>+ret = 0;
>+goto cleanup;
>+}
>+
>+virBufferEscapeRegex(, "^%s", cfg->channelTargetDir);
>+virBufferAddLit(, "/([^/]+\\.)|(domain-[^/]+/)");
>+virBufferEscapeRegex(, "%s$", chr->target.name);
>+
>+if (virBufferCheckError() < 0)
>+goto cleanup;
>+

No need to do this ^^, [1]

>+regexp = virBufferContentAndReset();
>+

[1] Just do this:

if ((regexp = virBufferContentAndReset()) < 0)
  goto cleanup;

or similar.


It's not equivalent, the 

[libvirt] [PATCH] maint: update to latest gnulib

2017-05-15 Thread Daniel P. Berrange
This pulls in the fixes for poll() on Win32 which finally
makes the remote driver work again.

Signed-off-by: Daniel P. Berrange 
---
 .gnulib | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.gnulib b/.gnulib
index 94386a1..da830b5 16
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit 94386a13667c645fd42544a7fd302c39fcdf
+Subproject commit da830b5146cb553ac2a4bcfe76caeb57bda24cc3
-- 
2.9.3

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


Re: [libvirt] [PATCH v2 11/38] Introduce virStreamSkip

2017-05-15 Thread Michal Privoznik
On 05/15/2017 10:56 AM, Daniel P. Berrange wrote:
> On Mon, May 15, 2017 at 10:54:03AM +0200, Michal Privoznik wrote:

> 
>> Now, question is whether we want signed or unsigned long long. I don't
>> have an opinion about that. On one hand, off_t is signed, but that's
>> because lseek() can seek backwards. We don't have that in our streams.
>> Yet. On the other hand, our streams are different to regular files. I
>> view them as a unidirectional pipe. With some extensions (e.g. sparse
>> messages). lseek() doesn't work over pipes, does it. But then again,
>> long long might be more future proof, if we will ever want to assign a
>> meaning to negative seeks.
> 
> I guess we might as well use signed long long - we aren't gaining anything
> by using unsigned long long, since the value will be truncated to signed
> long long when we use it with lseek().

Good point. I'll go with signed long long then.

Michal

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


Re: [libvirt] [PATCH v2 11/38] Introduce virStreamSkip

2017-05-15 Thread Daniel P. Berrange
On Mon, May 15, 2017 at 10:54:03AM +0200, Michal Privoznik wrote:
> On 05/15/2017 10:25 AM, Daniel P. Berrange wrote:
> > On Fri, May 12, 2017 at 09:29:27AM +0200, Michal Privoznik wrote:
> >> On 05/05/2017 04:48 PM, Daniel P. Berrange wrote:
> >>> On Fri, May 05, 2017 at 01:25:34PM +0200, Michal Privoznik wrote:
>  On 05/04/2017 11:29 PM, John Ferlan wrote:
> >
> >
> > On 04/20/2017 06:01 AM, Michal Privoznik wrote:
> >> This API can be used to tell the other side of the stream to skip
> >
> > s/can be/is  (unless it can be used for something else ;-))
> >
> >> some bytes in the stream. This can be used to create a sparse
> >> file on the receiving side of a stream.
> >>
> >> It takes just one argument @length, which says how big the hole
> >> is. Since our streams are not rewindable like regular files, we
> >> don't need @whence argument like seek(2) has.
> >
> > lseek is an implementation detail...  However, it could be stated that
> > the skipping would be from the current point in the file forward by some
> > number of bytes.  It's expected to be used in conjunction with code that
> > is copying over the real (or non-zero) data and should be considered an
> > optimization over sending zere data segments.
> >
> >>
> >> Signed-off-by: Michal Privoznik 
> >> ---
> >>  include/libvirt/libvirt-stream.h |  3 +++
> >>  src/driver-stream.h  |  5 
> >>  src/libvirt-stream.c | 57 
> >> 
> >>  src/libvirt_public.syms  |  1 +
> >>  4 files changed, 66 insertions(+)
> >>
> >
> > While it would be unused for now, should @flags be added.  Who knows
> > what use it could have, but avoids a new Flags API, but does cause a few
> > other wording changes here.
> 
>  Ah sure. We should have @flags there. Good point.
> 
> >
> > Perhaps it's just me - but "Skip" and "HoleSize" just seem awkward.
> > Would "virStreamSetSkip" and "virStreamGetSkip" be more apropos? (or
> > s/Skip/HoleSize/ - ewww). Names would then follow our more recent
> > function naming guidelines. I think I dislike the HoleSize much more
> > than the Skip.
> 
>  SetSkip and GetSkip sound wrong to me instead :D
> 
> >
> >> diff --git a/include/libvirt/libvirt-stream.h 
> >> b/include/libvirt/libvirt-stream.h
> >> index bee2516..4e0a599 100644
> >> --- a/include/libvirt/libvirt-stream.h
> >> +++ b/include/libvirt/libvirt-stream.h
> >> @@ -50,6 +50,9 @@ int virStreamRecvFlags(virStreamPtr st,
> >> size_t nbytes,
> >> unsigned int flags);
> >>  
> >> +int virStreamSkip(virStreamPtr st,
> >> +  unsigned long long length);
> >
> > Was there consideration for using 'off_t' instead of ULL? I know it's an
> > implementation detail of virFDStreamData and lseek() usage, but it does
> > hide things... IDC either way.
> 
>  The problem with off_t is that it is signed type, while ULL is unsigned.
>  There's not much point in sending a negative offset, is there?
>  Moreover, we use ULL for arguments like offset (not sure really why).
>  Frankly, I don't really know why. Perhaps some types don't exist 
>  everywhere?
> >>>
> >>> If anything, we would use  size_t, for consistency with the Send/Recv
> >>> methods. 
> >>
> >> So I've given this some though and ran some experiments. On a 32bit arch
> >> I've found this:
> >>
> >> long long 8 signed
> >> size_t 4 unsigned
> >> ssize_t 4 signed
> >> off_t 4 signed
> >>
> >> So size_t is 4 bytes long and long long is 8 bytes. This got me
> >> thinking, size_t type makes sense for those APIs where we need to
> >> address individual bytes. But what would happen if I have the following
> >> file on a 32 bit arch:
> >>
> >> [2MB data] -> [5GB hole] -> [2M data]
> >>
> >> The hole does not fit into size_t, but it would fit into long long. On
> >> the other hand, we are very likely to hit lseek() error as off_t is 4
> >> bytes also (WTF?!). On a 64 bit arch everything is as expected:
> > 
> > Were you testing libvirt, or testing a standalone demo program ?
> 
> Standalone demo program, that basically does this:
> 
> #define PRINT_SIZE(x)   printf( #x " %d %s\n", sizeof(x), (x)-1 < 0 ?
> "signed" : "unsigned")
> 
> PRINT_SIZE(long long);
> PRINT_SIZE(size_t);
> PRINT_SIZE(off_t);
> 
> 
> > 
> > Libvirt should be defining the macro that enables large file support,
> > which turns off_t into a 64-bit type.  So in fact, we would actually
> > be wrong to use size_t - off_t or long long is actually what we need
> > here.
> 
> I think we really need just long long. Consider that even though libvirt
> enables large file support internally, we don't enable it at the header
> files level. It's responsibility of 

Re: [libvirt] [PATCH v2 11/38] Introduce virStreamSkip

2017-05-15 Thread Michal Privoznik
On 05/15/2017 10:25 AM, Daniel P. Berrange wrote:
> On Fri, May 12, 2017 at 09:29:27AM +0200, Michal Privoznik wrote:
>> On 05/05/2017 04:48 PM, Daniel P. Berrange wrote:
>>> On Fri, May 05, 2017 at 01:25:34PM +0200, Michal Privoznik wrote:
 On 05/04/2017 11:29 PM, John Ferlan wrote:
>
>
> On 04/20/2017 06:01 AM, Michal Privoznik wrote:
>> This API can be used to tell the other side of the stream to skip
>
> s/can be/is  (unless it can be used for something else ;-))
>
>> some bytes in the stream. This can be used to create a sparse
>> file on the receiving side of a stream.
>>
>> It takes just one argument @length, which says how big the hole
>> is. Since our streams are not rewindable like regular files, we
>> don't need @whence argument like seek(2) has.
>
> lseek is an implementation detail...  However, it could be stated that
> the skipping would be from the current point in the file forward by some
> number of bytes.  It's expected to be used in conjunction with code that
> is copying over the real (or non-zero) data and should be considered an
> optimization over sending zere data segments.
>
>>
>> Signed-off-by: Michal Privoznik 
>> ---
>>  include/libvirt/libvirt-stream.h |  3 +++
>>  src/driver-stream.h  |  5 
>>  src/libvirt-stream.c | 57 
>> 
>>  src/libvirt_public.syms  |  1 +
>>  4 files changed, 66 insertions(+)
>>
>
> While it would be unused for now, should @flags be added.  Who knows
> what use it could have, but avoids a new Flags API, but does cause a few
> other wording changes here.

 Ah sure. We should have @flags there. Good point.

>
> Perhaps it's just me - but "Skip" and "HoleSize" just seem awkward.
> Would "virStreamSetSkip" and "virStreamGetSkip" be more apropos? (or
> s/Skip/HoleSize/ - ewww). Names would then follow our more recent
> function naming guidelines. I think I dislike the HoleSize much more
> than the Skip.

 SetSkip and GetSkip sound wrong to me instead :D

>
>> diff --git a/include/libvirt/libvirt-stream.h 
>> b/include/libvirt/libvirt-stream.h
>> index bee2516..4e0a599 100644
>> --- a/include/libvirt/libvirt-stream.h
>> +++ b/include/libvirt/libvirt-stream.h
>> @@ -50,6 +50,9 @@ int virStreamRecvFlags(virStreamPtr st,
>> size_t nbytes,
>> unsigned int flags);
>>  
>> +int virStreamSkip(virStreamPtr st,
>> +  unsigned long long length);
>
> Was there consideration for using 'off_t' instead of ULL? I know it's an
> implementation detail of virFDStreamData and lseek() usage, but it does
> hide things... IDC either way.

 The problem with off_t is that it is signed type, while ULL is unsigned.
 There's not much point in sending a negative offset, is there?
 Moreover, we use ULL for arguments like offset (not sure really why).
 Frankly, I don't really know why. Perhaps some types don't exist 
 everywhere?
>>>
>>> If anything, we would use  size_t, for consistency with the Send/Recv
>>> methods. 
>>
>> So I've given this some though and ran some experiments. On a 32bit arch
>> I've found this:
>>
>> long long 8 signed
>> size_t 4 unsigned
>> ssize_t 4 signed
>> off_t 4 signed
>>
>> So size_t is 4 bytes long and long long is 8 bytes. This got me
>> thinking, size_t type makes sense for those APIs where we need to
>> address individual bytes. But what would happen if I have the following
>> file on a 32 bit arch:
>>
>> [2MB data] -> [5GB hole] -> [2M data]
>>
>> The hole does not fit into size_t, but it would fit into long long. On
>> the other hand, we are very likely to hit lseek() error as off_t is 4
>> bytes also (WTF?!). On a 64 bit arch everything is as expected:
> 
> Were you testing libvirt, or testing a standalone demo program ?

Standalone demo program, that basically does this:

#define PRINT_SIZE(x)   printf( #x " %d %s\n", sizeof(x), (x)-1 < 0 ?
"signed" : "unsigned")

PRINT_SIZE(long long);
PRINT_SIZE(size_t);
PRINT_SIZE(off_t);


> 
> Libvirt should be defining the macro that enables large file support,
> which turns off_t into a 64-bit type.  So in fact, we would actually
> be wrong to use size_t - off_t or long long is actually what we need
> here.

I think we really need just long long. Consider that even though libvirt
enables large file support internally, we don't enable it at the header
files level. It's responsibility of developers of mgmt application to do
that. Having said that, if we had off_t in public API we can't possibly
know its size as it depends on something out of our control. Therefore,
I see long long as our only option.

Now, question is whether we want signed or unsigned long long. I don't
have an 

Re: [libvirt] [PATCH] RFE: virsh: add domxml-to-native [--domain DOMAIN] option

2017-05-15 Thread Dan
On Mon, Apr 24, 2017 at 09:17:12AM +0200, Peter Krempa wrote:
> On Sun, Apr 23, 2017 at 20:54:47 -0400, Dan wrote:
> 
> Please use your full name for patch submissions.
> 
I just did a new send-email patch submission to the list. Hopefully it
corrected my previous mistakes.
> > Bug 835476 RFE: virsh: add domxml-to-native --domain option (for existing
> > VM) [1]
> > 
> > virsh DOMAIN COMMAND domxml-to-native did not support domain (id|uuid|name)
> > as input for generating hypervisor agent native command, instead only
> > supported
> > XML input from STDIN. Here in this patch, it supports the following syntax:
> > domxml-to-native  { [--domain DOMAIN] | [XML] }, i.e., it supports
> > either designating domain (domain id, uuid, or name), or path to XML domain
> > configuration file; NOTE that it deprecated existing STDIN input passing XML
> > functionality. It was tested on the test mock driver and QEMU.
> > 
> > 
> > 
> > 
> > [1]. https://bugzilla.redhat.com/show_bug.cgi?id=835476
> > 
> > 
> > 
> > 
> > Dan
> 
> This whole block would get added to the commit message. You should not
> put anything into it which you don't want there. You want to describe
> the change briefly, but that's it.
> 
> 
> > 
> > ---
> >  tools/virsh-domain.c | 68
> > ++--
> >  1 file changed, 55 insertions(+), 13 deletions(-)
> 
> This patch is corrupted. Usually it's the best to use git-send-email to
> post patches. Alternatively you can attach the file itself. Pasting the
> patch into your mail client will corrupt it most probably.
> 
> Please re-send a non-corrupted version with your proper name as the
> author.
>
I configured my git send-email. Please let me know if there is still
format issues.

> Also if you didn't make so, make sure you run make check and make
> syntax-check before posting patches.
> 
This time I ran both of the checks, except that "make check" skipped
test.
> > 
> > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> > index db8accfe4..9ac855b19 100644
> > --- a/tools/virsh-domain.c
> > +++ b/tools/virsh-domain.c
> > @@ -60,6 +60,7 @@
> >  #include "virsh-nodedev.h"
> >  #include "viruri.h"
> > 
> > +
> 
> Spurious whitespace addition.
> 
I deleted it. Thank you very much for pointing it out.
> >  /* Gnulib doesn't guarantee SA_SIGINFO support.  */
> >  #ifndef SA_SIGINFO
> >  # define SA_SIGINFO 0
> > @@ -9811,9 +9812,13 @@ static const vshCmdOptDef opts_domxmltonative[] = {
> >   .flags = VSH_OFLAG_REQ,
> >   .help = N_("target config data type format")
> >  },
> > +{.name = "domain",
> > + .type = VSH_OT_DATA,
> > + .flags = VSH_OFLAG_REQ_OPT,
> > + .help = N_("domain name, id or uuid")
> > +},
> >  {.name = "xml",
> >   .type = VSH_OT_DATA,
> > - .flags = VSH_OFLAG_REQ,
> >   .help = N_("xml data file to export from")
> >  },
> >  {.name = NULL}
> > @@ -9822,31 +9827,68 @@ static const vshCmdOptDef opts_domxmltonative[] = {
> >  static bool
> >  cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd)
> >  {
> > -bool ret = true;
> >  const char *format = NULL;
> >  const char *xmlFile = NULL;
> > -char *configData;
> > -char *xmlData;
> > +const char *domain = NULL;
> > +char *configData = NULL;
> > +char *xmlData = NULL;
> >  unsigned int flags = 0;
> > +unsigned int domflags = 0;
> >  virshControlPtr priv = ctl->privData;
> > +virDomainPtr dom = NULL;
> > 
> > -if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0 ||
> > -vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
> > +if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0)
> >  return false;
> > +
> > +if (vshCommandOptStringReq(ctl, cmd, "domain", ) < 0)
> > + return false;
> > 
> > -if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, ) < 0)
> > -return false;
> > +if (vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
> > + return false;
> > +
> > +VSH_EXCLUSIVE_OPTIONS_VAR(domain, xmlFile);
> > +
> > +if (domain) {
> > + domflags = VIRSH_BYID | VIRSH_BYUUID | VIRSH_BYNAME;
> > +dom = virshLookupDomainBy(ctl, domain, domflags);
> 
> You can use virshCommandOptDomain instead of this. And the string
> lookup.
> 
Oh yeah, I used it this time. I would not know its existence without you
telling me. So many options to achieve one thing. But this seems to be
the most elegant way.

Thanks a lot,

Dan
> > +}
> > +
> > +if (!dom && !xmlFile) {


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


[libvirt] [PATCH] virsh: add --domain option for domain-to-native

2017-05-15 Thread Daniel Liu
Fix bug 835476[1].
virsh: add [--domain DOMAIN] option to  domxml-to-native DOMAIN COMMAND
Add support for the following syntax:
domxml-to-native  { [--domain DOMAIN] | [XML] }, i.e., it supports
either designating domain (domain id, uuid, or name), or path to XML domain
configuration file.

E.g.:
virsh domxml-to-native qemu-argv --domain RHEL7.3   # domain name
virsh domxml-to-native qemu-argv --domain 10# domain id
virsh domxml-to-native qemu-argv dumped_dom.xml # dumped xml

[1] https://bugzilla.redhat.com/show_bug.cgi?id=835476
---
 tools/virsh-domain.c | 54 ++--
 1 file changed, 44 insertions(+), 10 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 0d19d0e01..a79fd3ab2 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9840,9 +9840,13 @@ static const vshCmdOptDef opts_domxmltonative[] = {
  .flags = VSH_OFLAG_REQ,
  .help = N_("target config data type format")
 },
+{.name = "domain",
+ .type = VSH_OT_DATA,
+ .flags = VSH_OFLAG_REQ_OPT,
+ .help = N_("domain name, id or uuid")
+},
 {.name = "xml",
  .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
  .help = N_("xml data file to export from")
 },
 {.name = NULL}
@@ -9851,30 +9855,60 @@ static const vshCmdOptDef opts_domxmltonative[] = {
 static bool
 cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd)
 {
-bool ret = true;
+bool ret = false;
 const char *format = NULL;
-const char *xmlFile = NULL;
-char *configData;
-char *xmlData;
+const char *domain = NULL;
+const char *xml = NULL;
+char *xmlData = NULL;
+char *configData = NULL;
 unsigned int flags = 0;
 virshControlPtr priv = ctl->privData;
+virDomainPtr dom = NULL;
 
-if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0 ||
-vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
+if (vshCommandOptStringReq(ctl, cmd, "format", ) < 0)
+return false;
+
+if (vshCommandOptStringReq(ctl, cmd, "domain", ) < 0)
+return false;
+
+if (vshCommandOptStringReq(ctl, cmd, "xml", ) < 0)
 return false;
 
-if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, ) < 0)
+VSH_EXCLUSIVE_OPTIONS_VAR(domain, xml);
+
+if (domain)
+dom = virshCommandOptDomain(ctl, cmd, );
+
+if (!dom && !xml) {
+vshError(ctl, _("need either domain (ID, UUID, or name) or domain XML 
configuration file path"));
 return false;
+}
+
+if (dom) {
+xmlData = virDomainGetXMLDesc(dom, flags);
+if (xmlData == NULL)
+goto cleanup;
+}
+
+if (xml) {
+if (virFileReadAll(xml, VSH_MAX_XML_FILE, ) < 0)
+goto cleanup;
+}
 
 configData = virConnectDomainXMLToNative(priv->conn, format, xmlData, 
flags);
 if (configData != NULL) {
 vshPrint(ctl, "%s", configData);
-VIR_FREE(configData);
+ret = true;
+goto cleanup;
 } else {
-ret = false;
+vshError(ctl, _("convert from domain XML to native command failed"));
+goto cleanup;
 }
 
+ cleanup:
+virshDomainFree(dom);
 VIR_FREE(xmlData);
+VIR_FREE(configData);
 return ret;
 }
 
-- 
2.13.0

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


Re: [libvirt] [PATCH] storage: use 0711 as the default perms for dirs

2017-05-15 Thread Daniel P. Berrange
On Thu, May 11, 2017 at 06:36:22PM -0400, John Ferlan wrote:
> 
> 
> On 05/11/2017 04:31 AM, Christian Ehrhardt wrote:
> > From: Serge Hallyn 
> > 
> > There should be no need to make dir based pools world readable.
> > So use 0711, not 0755, as the default perms for storage dirs.
> > 
> > Signed-off-by: Christian Ehrhardt 
> > ---
> >  docs/formatstorage.html.in | 2 +-
> >  src/storage/storage_util.h | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> 
> Kinda surprised this didn't generate some immediate discussion...  I
> would also think that if you had a desire to change defaults you'd also
> have a libvirt.spec.in adjustment...

Actually no it doesn't - the spec file is already marking
/var/lib/libvirt/images as 0711.

> Still 0755 or umask(022) seem to be fairly prevalent setting and having
> the  for the XML to be able to override a default certainly gives
> credence to arguments in either direction whether or not to change the
> defaults.
> 
> It's been a long while since I considered system/directory/file security
> things, but I have this faint recollection of some strange issue when
> not having world or group "executable" as a default.

The fact that RPM spec ships with 0711 show that it works ok. So I
think this change is reasonable.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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


Re: [libvirt] [PATCH v2 11/38] Introduce virStreamSkip

2017-05-15 Thread Daniel P. Berrange
On Fri, May 12, 2017 at 09:29:27AM +0200, Michal Privoznik wrote:
> On 05/05/2017 04:48 PM, Daniel P. Berrange wrote:
> > On Fri, May 05, 2017 at 01:25:34PM +0200, Michal Privoznik wrote:
> >> On 05/04/2017 11:29 PM, John Ferlan wrote:
> >>>
> >>>
> >>> On 04/20/2017 06:01 AM, Michal Privoznik wrote:
>  This API can be used to tell the other side of the stream to skip
> >>>
> >>> s/can be/is  (unless it can be used for something else ;-))
> >>>
>  some bytes in the stream. This can be used to create a sparse
>  file on the receiving side of a stream.
> 
>  It takes just one argument @length, which says how big the hole
>  is. Since our streams are not rewindable like regular files, we
>  don't need @whence argument like seek(2) has.
> >>>
> >>> lseek is an implementation detail...  However, it could be stated that
> >>> the skipping would be from the current point in the file forward by some
> >>> number of bytes.  It's expected to be used in conjunction with code that
> >>> is copying over the real (or non-zero) data and should be considered an
> >>> optimization over sending zere data segments.
> >>>
> 
>  Signed-off-by: Michal Privoznik 
>  ---
>   include/libvirt/libvirt-stream.h |  3 +++
>   src/driver-stream.h  |  5 
>   src/libvirt-stream.c | 57 
>  
>   src/libvirt_public.syms  |  1 +
>   4 files changed, 66 insertions(+)
> 
> >>>
> >>> While it would be unused for now, should @flags be added.  Who knows
> >>> what use it could have, but avoids a new Flags API, but does cause a few
> >>> other wording changes here.
> >>
> >> Ah sure. We should have @flags there. Good point.
> >>
> >>>
> >>> Perhaps it's just me - but "Skip" and "HoleSize" just seem awkward.
> >>> Would "virStreamSetSkip" and "virStreamGetSkip" be more apropos? (or
> >>> s/Skip/HoleSize/ - ewww). Names would then follow our more recent
> >>> function naming guidelines. I think I dislike the HoleSize much more
> >>> than the Skip.
> >>
> >> SetSkip and GetSkip sound wrong to me instead :D
> >>
> >>>
>  diff --git a/include/libvirt/libvirt-stream.h 
>  b/include/libvirt/libvirt-stream.h
>  index bee2516..4e0a599 100644
>  --- a/include/libvirt/libvirt-stream.h
>  +++ b/include/libvirt/libvirt-stream.h
>  @@ -50,6 +50,9 @@ int virStreamRecvFlags(virStreamPtr st,
>  size_t nbytes,
>  unsigned int flags);
>   
>  +int virStreamSkip(virStreamPtr st,
>  +  unsigned long long length);
> >>>
> >>> Was there consideration for using 'off_t' instead of ULL? I know it's an
> >>> implementation detail of virFDStreamData and lseek() usage, but it does
> >>> hide things... IDC either way.
> >>
> >> The problem with off_t is that it is signed type, while ULL is unsigned.
> >> There's not much point in sending a negative offset, is there?
> >> Moreover, we use ULL for arguments like offset (not sure really why).
> >> Frankly, I don't really know why. Perhaps some types don't exist 
> >> everywhere?
> > 
> > If anything, we would use  size_t, for consistency with the Send/Recv
> > methods. 
> 
> So I've given this some though and ran some experiments. On a 32bit arch
> I've found this:
> 
> long long 8 signed
> size_t 4 unsigned
> ssize_t 4 signed
> off_t 4 signed
> 
> So size_t is 4 bytes long and long long is 8 bytes. This got me
> thinking, size_t type makes sense for those APIs where we need to
> address individual bytes. But what would happen if I have the following
> file on a 32 bit arch:
> 
> [2MB data] -> [5GB hole] -> [2M data]
> 
> The hole does not fit into size_t, but it would fit into long long. On
> the other hand, we are very likely to hit lseek() error as off_t is 4
> bytes also (WTF?!). On a 64 bit arch everything is as expected:

Were you testing libvirt, or testing a standalone demo program ?

Libvirt should be defining the macro that enables large file support,
which turns off_t into a 64-bit type.  So in fact, we would actually
be wrong to use size_t - off_t or long long is actually what we need
here.

I was wrong about consistency with Send/Recv, since the arg there is
about the length of the data array which is size_t and this is a
different boundary than max file size which is off_t.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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


Re: [libvirt] [PATCH] util: conf: Don't log when adding commented out lines

2017-05-15 Thread Peter Krempa
On Fri, May 12, 2017 at 16:43:05 +0200, Martin Kletzander wrote:
> On Fri, May 12, 2017 at 04:33:29PM +0200, Peter Krempa wrote:
> > virConfAddEntry spams debug logs even for fully commented out lines.
> > Skip such messages to avoid:
> > 
> > 2017-05-12 12:35:38.867+: 10820: debug : virConfAddEntry:241 : Add 
> > entry (null) (nil)
> > 2017-05-12 12:35:38.867+: 10820: debug : virConfAddEntry:241 : Add 
> > entry (null) (nil)
> > 2017-05-12 12:35:38.867+: 10820: debug : virConfAddEntry:241 : Add 
> > entry (null) (nil)
> > 2017-05-12 12:35:38.867+: 10820: debug : virConfAddEntry:241 : Add 
> > entry (null) (nil)
> > 2017-05-12 12:35:38.867+: 10820: debug : virConfAddEntry:241 : Add 
> > entry (null) (nil)
> > ...
> > 
> > This also fixes NULL passed to printf.
> > ---
> > src/util/virconf.c | 5 -
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/util/virconf.c b/src/util/virconf.c
> > index 9840ca6c7..4498d253a 100644
> > --- a/src/util/virconf.c
> > +++ b/src/util/virconf.c
> > @@ -238,7 +238,10 @@ virConfAddEntry(virConfPtr conf, char *name, 
> > virConfValuePtr value, char *comm)
> > if ((comm == NULL) && (name == NULL))
> > return NULL;
> > 
> > -VIR_DEBUG("Add entry %s %p", name, value);
> > +/* don't log fully commented out lines */
> > +if (name)
> > +VIR_DEBUG("Add entry %s %p", name, value);
> > +
> 
> Probably better then adding the comment to the debug message.

Yes, I tried that at first. It was awful.

> 
> ACK

Thanks


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