Re: [libvirt] [PATCH v2] qemu: Adding 'downscript' feature for QEMU network interfaces.
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
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
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
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
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
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
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
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
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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
From: Serge HallynUpdates 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
From: Simon McVittieThe 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
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 EhrhardtSigned-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.
From: William GrantAllow 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
From: Serge HallynAdd 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
From: Felix GeyerAllow 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
From: Christian EhrhardtThis 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
From: Felix GeyerLocal 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
From: Serge HallynJust 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
From: Felix GeyerAdd 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)
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
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
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
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
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
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
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
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
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
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
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
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
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
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
On Mon, May 15, 2017 at 1:10 PM, Daniel P. Berrangewrote: > 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
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
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
From: Serge HallynThere 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
On Fri, May 12, 2017 at 12:36 AM, John Ferlanwrote: > 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
On Mon, May 15, 2017 at 10:27 AM, Daniel P. Berrangewrote: > > 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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