Re: [libvirt] running Libvirt from source code, IPC_LOCK and VFIO
Update: I've figured it out. The bug here was that, even running as root, I was getting errors like: error : virQEMUCapsNewForBinaryInternal:4687 : internal error: Failed to probe QEMU binary with QMP: libvirt: error : prctl failed to enable 'dac_override' in the AMBIENT set: Operation not permitted The reason is that the host has libcap-ng installed. ./configure uses it if available, setting WITH_CAPNG in the code. I am unsure if this has something to do with the libcap-ng configuration in this system I'm using or if there is something missing in the Libvirt code, but the spawned QEMU process isn't inheriting the capabilities it should have. Disabling support of this lib with "--with-capng=no" in autogen.sh and rebuilding Libvirt fixed the problem. I was even able to see more NUMA nodes than I was before using the system libvirt (which is the original bug I am/was investigating). Thanks! On 2/1/19 4:04 PM, Daniel Henrique Barboza wrote: Hi, I'm facing a strange behavior when running Libvirt from source code, latest upstream, on an Ubuntu 18.04.1 LTS Power 9 server. My QEMU guest - which is using VFIO and GPU passthrough - breaks on boot when trying to allocate a DMA window inside KVM. Debugging the code, I've found out that the problem is related to the process not having CAP_IPC_LOCK - at least from the host kernel perspective. This is strange because: - the same VM running directly from QEMU command line works - the same VM running in the system Libvirt (v4.0.0, Ubuntu version) also works What am I missing? My understanding on Linux process is that a process running as root should inherit the same capabilities of the user, which includes CAP_IPC_LOCK. Running Libvirt from source code should grant ipc_lock to it ... right? Any help is appreciated. I can provide more details (VM XML for example) if necessary. Thanks! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] bhyve: bhyve:commandline followup fixes
Daniel P. Berrangé wrote: > On Thu, Jan 31, 2019 at 04:41:23PM +0400, Roman Bogorodskiy wrote: > > Roman Bogorodskiy (3): > > bhyve: bhyveDomainDefNamespaceFormatXML cleanup > > bhyve: emit warning when using bhyve:commandline > > docs: bhyve: warn about bhyve:commandline risks > > > > docs/drvbhyve.html.in | 5 + > > src/bhyve/bhyve_command.c | 4 > > src/bhyve/bhyve_domain.c | 4 ++-- > > 3 files changed, 11 insertions(+), 2 deletions(-) > > For all three patches > > Reviewed-by: Daniel P. Berrangé > > 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 :| Pushed, thanks! Roman Bogorodskiy signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH v2] qemu-nbd: Deprecate qemu-nbd --partition
On 1/25/19 5:48 PM, Eric Blake wrote: > The existing qemu-nbd --partition code claims to handle logical > partitions up to 8, since its introduction in 2008 (commit 7a5ca86). > However, the implementation is bogus (actual MBR logical partitions > form a sort of linked list, with one partition per extended table > entry, rather than four logical partitions in a single extended > table), making the code unlikely to work for anything beyond -P5 on > actual guest images. What's more, the code does not support GPT > partitions, which are becoming more popular, and maintaining device > subsetting in both NBD and the raw device is unnecessary duplication > of effort (even if it is not too difficult). > > Note that obtaining the offsets of a partition (MBR or GPT) can be > learned by using 'qemu-nbd -c /dev/nbd0 file.qcow2 && sfdisk --dump > /dev/nbd0', but by the time you've done that, you might as well > just mount /dev/nbd0p1 that the kernel creates for you instead of > bothering with qemu exporting a subset. Or, keeping to just > user-space code, use nbdkit's partition filter, which has already > known both GPT and primary MBR partitions for a while, and was > just recently enhanced to support arbitrary logical MBR parititions. > > Start the clock on the deprecation cycle, with examples of how > to write device subsetting without using -P. > > Signed-off-by: Eric Blake > > --- > v2: actual nbdkit example [Rich], improved doc wording Thanks; queued for my next NBD pull request. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] running Libvirt from source code, IPC_LOCK and VFIO
Hi, I'm facing a strange behavior when running Libvirt from source code, latest upstream, on an Ubuntu 18.04.1 LTS Power 9 server. My QEMU guest - which is using VFIO and GPU passthrough - breaks on boot when trying to allocate a DMA window inside KVM. Debugging the code, I've found out that the problem is related to the process not having CAP_IPC_LOCK - at least from the host kernel perspective. This is strange because: - the same VM running directly from QEMU command line works - the same VM running in the system Libvirt (v4.0.0, Ubuntu version) also works What am I missing? My understanding on Linux process is that a process running as root should inherit the same capabilities of the user, which includes CAP_IPC_LOCK. Running Libvirt from source code should grant ipc_lock to it ... right? Any help is appreciated. I can provide more details (VM XML for example) if necessary. Thanks! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: command: Don't skip 'readonly' and throttling info for empty drive
On Fri, Feb 01, 2019 at 06:00:37PM +0100, Peter Krempa wrote: > In commit f80eae8c2ae I was too agresive in removing properties of > -drive for empty drives. It turns out that qemu actually persists the > state of 'readonly' and the throttling information even for the empty > drive. > > Removing 'readonly' thus made qemu open any subsequent images added via > the 'change' command as RW which was forbidden by selinux thanks to the > restrictive sVirt label for readonly media. > > Fix this by formating the property again and bump the tests and leave a > note detailing why the rest of the properties needs to be skipped. > > Signed-off-by: Peter Krempa > --- > src/qemu/qemu_command.c| 18 +- > tests/qemuxml2argvdata/disk-cdrom.args | 4 ++-- > .../disk-cdrom.x86_64-2.12.0.args | 4 ++-- > .../disk-cdrom.x86_64-latest.args | 4 ++-- > 4 files changed, 19 insertions(+), 11 deletions(-) Reviewed-by: Daniel P. Berrangé 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] util: Fix build issue with virStorageFileGetNPIVKey
Signed-off-by: John Ferlan
---
pushed as build breaker ...
src/util/virstoragefile.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index d83d84fcf5..6df0885669 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1564,7 +1564,7 @@ virStorageFileGetNPIVKey(const char *path,
return ret;
}
#else
-int virStorageFileGetNPIVKey(const char *path,
+int virStorageFileGetNPIVKey(const char *path ATTRIBUTE_UNUSED,
char **key ATTRIBUTE_UNUSED)
{
return -1;
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: command: Don't skip 'readonly' and throttling info for empty drive
In commit f80eae8c2ae I was too agresive in removing properties of
-drive for empty drives. It turns out that qemu actually persists the
state of 'readonly' and the throttling information even for the empty
drive.
Removing 'readonly' thus made qemu open any subsequent images added via
the 'change' command as RW which was forbidden by selinux thanks to the
restrictive sVirt label for readonly media.
Fix this by formating the property again and bump the tests and leave a
note detailing why the rest of the properties needs to be skipped.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_command.c| 18 +-
tests/qemuxml2argvdata/disk-cdrom.args | 4 ++--
.../disk-cdrom.x86_64-2.12.0.args | 4 ++--
.../disk-cdrom.x86_64-latest.args | 4 ++--
4 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a59583fb75..6d3aa69569 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1764,10 +1764,18 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
}
}
+if (disk->src->readonly)
+virBufferAddLit(, ",readonly=on");
+
+/* qemu rejects some parameters for an empty -drive, so we need to skip
+ * them in that case:
+ * cache: modifies properties of the format driver which is not present
+ * copy_on_read: really only works for floppies
+ * discard: modifies properties of format driver
+ * detect_zeroes: works but really depends on discard so it's useless
+ * iomode: setting it to 'native' requires a specific cache mode
+ */
if (!virStorageSourceIsEmpty(disk->src)) {
-if (disk->src->readonly)
-virBufferAddLit(, ",readonly=on");
-
if (disk->cachemode) {
virBufferAsprintf(, ",cache=%s",
qemuDiskCacheV2TypeToString(disk->cachemode));
@@ -1792,10 +1800,10 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
virBufferAsprintf(, ",aio=%s",
virDomainDiskIoTypeToString(disk->iomode));
}
-
-qemuBuildDiskThrottling(disk, );
}
+qemuBuildDiskThrottling(disk, );
+
if (virBufferCheckError() < 0)
goto error;
diff --git a/tests/qemuxml2argvdata/disk-cdrom.args
b/tests/qemuxml2argvdata/disk-cdrom.args
index a9f60aa477..4823ae82de 100644
--- a/tests/qemuxml2argvdata/disk-cdrom.args
+++ b/tests/qemuxml2argvdata/disk-cdrom.args
@@ -27,7 +27,7 @@ bootindex=1 \
-drive file=/root/boot.iso,format=raw,if=none,id=drive-ide0-0-1,media=cdrom,\
readonly=on \
-device ide-drive,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \
--drive if=none,id=drive-ide0-1-0,media=cdrom \
+-drive if=none,id=drive-ide0-1-0,media=cdrom,readonly=on \
-device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \
--drive if=none,id=drive-ide0-1-1,media=cdrom \
+-drive if=none,id=drive-ide0-1-1,media=cdrom,readonly=on \
-device ide-drive,bus=ide.1,unit=1,drive=drive-ide0-1-1,id=ide0-1-1
diff --git a/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args
b/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args
index a39d920f67..2fe84177b8 100644
--- a/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args
+++ b/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args
@@ -28,10 +28,10 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
-drive file=/root/boot.iso,format=raw,if=none,id=drive-ide0-0-1,readonly=on \
-device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \
--drive if=none,id=drive-ide0-1-0 \
+-drive if=none,id=drive-ide0-1-0,readonly=on \
-device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0,\
write-cache=on \
--drive if=none,id=drive-ide0-1-1 \
+-drive if=none,id=drive-ide0-1-1,readonly=on \
-device ide-cd,bus=ide.1,unit=1,drive=drive-ide0-1-1,id=ide0-1-1 \
-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
resourcecontrol=deny \
diff --git a/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args
b/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args
index 029ae23dfa..9b9451f435 100644
--- a/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args
@@ -28,10 +28,10 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
-drive file=/root/boot.iso,format=raw,if=none,id=drive-ide0-0-1,readonly=on \
-device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \
--drive if=none,id=drive-ide0-1-0 \
+-drive if=none,id=drive-ide0-1-0,readonly=on \
-device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0,\
write-cache=on \
--drive if=none,id=drive-ide0-1-1 \
+-drive if=none,id=drive-ide0-1-1,readonly=on \
-device ide-cd,bus=ide.1,unit=1,drive=drive-ide0-1-1,id=ide0-1-1 \
-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
Re: [libvirt] [PATCH 3/3] qemu: domain: Treat 'volume' disks as 'raw' if neiter user nor pool provided format
On 1/31/19 8:42 AM, Peter Krempa wrote: Subject: s/neiter/neither/ Long line; maybe: qemu: domain: Use 'raw' for 'volume' disks without format > Storage pools might want to specify format of the image when translating > the volume thus we can't add any default format when parsing the XML. > > Add a explicit format when starting the VM and format is not present > neither by user specifying it nor by the storage pool translation > function. > > Signed-off-by: Peter Krempa > --- -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PULL 0/5] Ui 20190201 patches
On Fri, 1 Feb 2019 at 12:35, Gerd Hoffmann wrote: > > The following changes since commit e8977901b79fb678f288dd372261b640bbeccd0d: > > Merge remote-tracking branch > 'remotes/vivier2/tags/trivial-branch-pull-request' into staging (2019-01-31 > 15:40:39 +) > > are available in the git repository at: > > git://git.kraxel.org/qemu tags/ui-20190201-pull-request > > for you to fetch changes up to 0015ca5cbabe0b31d31610ddfaafd90a9e5911a4: > > ui: remove support for SDL1.2 in favour of SDL2 (2019-02-01 11:59:12 +0100) > > > ui: fix build with SDL disabled, drop SDL1 support. > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0 for any user-visible changes. -- PMM -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: news: Update the release notes with the SEV permission fix
On Fri, Feb 01, 2019 at 01:53:47PM +0100, Andrea Bolognani wrote: > On Fri, 2019-02-01 at 13:07 +0100, Erik Skultety wrote: > [...] > > + By default, libvirt runs the QEMU process as qemu:qemu which > > could > > + cause issues during probing as some features (like AMD SEV) > > might be > > + inaccesible to QEMU because of file system permissions. > > Therefore, > > + CAP_DAC_OVERRIDE is granted to overcome these for the purposes of > > + probing. > > You can wrap 'qemu:qemu' and 'CAP_DAC_OVERRIDE' in elements > so that they will look nicer in the HTML version. > > I'd also change ' (like AMD SEV)' to ', like AMD SEV,', but feel > free to leave it alone if you like it better that way. Fixed and pushed. Thanks, Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/7] configure: selectively install a firewalld 'libvirt' zone
On 2/1/19 8:49 AM, Laine Stump wrote: On 2/1/19 8:28 AM, Eric Garver wrote: On Thu, Jan 31, 2019 at 10:10:43PM -0500, Laine Stump wrote: On 1/31/19 8:24 PM, Laine Stump wrote: Changes from V1: [...] * make the rule's priority 32767 instead of 127. [...] + + + + I found out after sending this that when I make the priority of the reject rule 32767 instead of 127, it's apparently ignored (in my example, I was able to ssh to port 222 of the host even though the zone doesn't allow that). Eric, any idea why this might be happening? What build are you testing against? At one point the limit was 127, but I increased it before pushing it upstream. You can check the firewalld logs - there may be an error reporting the above priority is out of range. Ah, maybe you haven't backported that change to RHEL? I was testing on my RHEL8 beta system. If that's the case, then either we need that change backported to RHEL too, or I need to change the priority back to 127. Okay, Eric and I figured out thie problem was that my test machine was running an early scratch build of the firewalld package that had the limit for priority at 127, but also had been given a fake version that was *higher* than the proper build in the repo, so yum update wasn't grabbing it. Now that my firewalld package is up to date, it works properly! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] qemu: Improve handling of format for 'network' and 'volume' disks (blockdev-add saga)
On Thu, Jan 31, 2019 at 03:42:53PM +0100, Peter Krempa wrote: Peter Krempa (3): tests: qemu: Test network disks without format specified explicitly qemu: domain: Assume 'raw' default storage format also for network storage qemu: domain: Treat 'volume' disks as 'raw' if neiter user nor pool provided format src/qemu/qemu_domain.c| 8 ++-- tests/qemuxml2argvdata/disk-network-gluster.xml | 2 +- tests/qemuxml2argvdata/disk-network-iscsi.xml | 2 +- tests/qemuxml2argvdata/disk-network-nbd.xml | 2 +- tests/qemuxml2argvdata/disk-source-pool-mode.args | 6 +++--- tests/qemuxml2argvdata/disk-source-pool.args | 4 ++-- 6 files changed, 14 insertions(+), 10 deletions(-) Reviewed-by: Ján Tomko Jano 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 5/7] network: set firewalld zone of bridges to "libvirt" zone when appropriate
On Fri, Feb 01, 2019 at 10:10:21AM -0500, Laine Stump wrote: > On 2/1/19 8:24 AM, Daniel P. Berrangé wrote: > > On Thu, Jan 31, 2019 at 08:24:56PM -0500, Laine Stump wrote: > > > From: Laine Stump > > > > > > This patch restores broken guest network connectivity after a host > > > firewalld is switched to using an nftables backend. It does this by > > > adding libvirt networks' bridge interfaces to the new "libvirt" zone > > > in firewalld. > > > > > > After this patch, the bridge interface of any network created by > > > libvirt (when firewalld is active) will be added to the firewalld > > > zone called "libvirt" if it exists (regardless of the firewalld > > > backend setting). This behavior does *not* depend on whether or not > > > libvirt has installed the libvirt zone file (set with > > > "--with[out]-firewalld-zone" during the configure phase of the package > > > build). > > > > > > If the libvirt zone doesn't exist (either because the package was > > > configured to not install it, or possibly it was installed, but > > > firewalld doesn't support rule priorities, resulting in a parse > > > error), the bridge will remain in firewalld's default zone, which > > > could be innocuous (in the case that the firewalld backend is > > > iptables, guest networking will still function properly with the > > > bridge in the default zone), or it could be disastrous (if the > > > firewalld backend is nftables, we can be assured that guest networking > > > will fail). In order to be unobtrusive in the former case, and > > > informative in the latter, when the libvirt zone doesn't exist we > > > then check the firewalld version to see if it's new enough to support > > > the nftables backend, and then if the backend is actually set to > > > nftables, before logging an error (and failing the net-start > > > operation, since the network couldn't possibly work anyway). > > > > > > When the libvirt zone is used, network behavior is *slightly* > > > different from behavior of previous libvirt. In the past, libvirt > > > network behavior would be affected by the configuration of firewalld's > > > default zone (usually "public"), but now it is affected only by the > > > "libvirt" zone), and thus almost surely warrants a release note for > > > any distro upgrading to libvirt 5.1 or above. Although it's > > > unfortunate that we have to deal with a mandatory behavior change, the > > > architecture of multiple hooks makes it impossible to *not* change > > > behavior in some way, and the new behavior is arguably better (since > > > it will now be possible to manage access to the host from virtual > > > machines vs from public interfaces separately). > > > > > > Resolves: https://bugzilla.redhat.com/1638342 > > > Creates-and-Resolves: https://bugzilla.redhat.com/1650320 > > > Signed-off-by: Laine Stump > > > --- > > > > > > NB: I had considered that it might be useful to cache the results of > > > checking the list of active zones, the firewalld version, or the > > > firewalld backend, but in my tests of restarting libvirtd with 100 > > > active networks, the full startup time (from the beginning of > > > "systemctl restart libvirtd.service" until successful return from a > > > subsequent "virsh list") showed 42 seconds and a bit regardless of > > > whether or not I made those checks. This tells me that the amount of > > > time to be saved by caching the results of a single call vs calling > > > once for each network are insignificant relative to everything else > > > that is being done. Because any cached values would need to be stored > > > in the network driver state object, and thus require acquiring the > > > driver-wide lock to update at potentially very different times > > > (e.g. in the response to a dbus message informing us that firewalld > > > was restarted, as well as while starting a new network from an API > > > call) I consider the chance of a bug in my code causing an obscure > > > deadlock sometime in the future to be a much greater concern than > > > maybe saving 1/10th of a second out of 42 (and lock contention might > > > eliminate the gain anyway(), so I have left the code to retrieve the > > > list of zones once for each network start. > > > > > > Changes in V2: > > > > > > * split off from Patch 5. This patch only sets the libvirt zone if it > > >exists (and attempts to somewhat document the behavior in > > >firewall.html), it doesn't install the libvirt zone. > > > > > > * check for existence of libvirt zone before attempting to set it. > > > > > > * if libvirt zone doesn't exist, only log an error in the case that > > >the firewalld version is new enough to have an nftables backend, and > > >the backend is actually set to nftables. > > > > > > > > > docs/firewall.html.in | 33 + > > > src/network/bridge_driver_linux.c | 48 +++ > > > 2 files changed, 81 insertions(+) > > > > > > diff --git
Re: [libvirt] [PATCH v2 4/4] storage: Fetch a unique key for vHBA/NPIV LUNs
On Fri, Jan 18, 2019 at 09:42:37AM -0500, John Ferlan wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1657468 Commit be1bb6c95 changed the way volumes were stored from a forward linked list to a hash table. In doing so, it required that each vol object would have 3 unique values as keys into tables - key, name, and path. Due to how vHBA/NPIV LUNs are created/used this resulted in a failure to utilize all the LUN's found during processing. During virStorageBackendSCSINewLun processing fetch the key (or serial value) for NPIV LUN's using virStorageFileGetNPIVKey which will formulate a more unique key based on the serial value and the port for the LUN. Signed-off-by: John Ferlan --- src/storage/storage_util.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) Reviewed-by: Ján Tomko Jano 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/7] util: new virFirewallD APIs + docs
On Fri, Feb 01, 2019 at 10:06:44AM -0500, Laine Stump wrote:
> On 2/1/19 8:17 AM, Daniel P. Berrangé wrote:
> > On Thu, Jan 31, 2019 at 08:24:54PM -0500, Laine Stump wrote:
> > > +int
> > > +virFirewallDGetBackend(void)
> > > +{
> > > +DBusConnection *sysbus = virDBusGetSystemBus();
> > > +DBusMessage *reply = NULL;
> > > +virError error;
> > > +VIR_AUTOFREE(char *) backendStr = NULL;
> > > +int backend = -1;
> > > +
> > > +if (!sysbus)
> > > +return -1;
> > > +
> > > +memset(, 0, sizeof(error));
> > > +
> > > +if (virDBusCallMethod(sysbus,
> > > + ,
> > > + ,
> > > + VIR_FIREWALL_FIREWALLD_SERVICE,
> > > + "/org/fedoraproject/FirewallD1/config",
> > > + "org.freedesktop.DBus.Properties",
> > > + "Get",
> > > + "ss",
> > > + "org.fedoraproject.FirewallD1.config",
> > > + "FirewallBackend") < 0)
> > > +goto cleanup;
> > > +
> > > +if (error.level == VIR_ERR_ERROR) {
> > > +/* we don't want to log any error in the case that
> > > + * FirewallBackend isn't implemented in this firewalld, since
> > > + * that just means that it is an old version, and only has an
> > > + * iptables backend.
> > > + */
> > > +VIR_DEBUG("Failed to get FirewallBackend setting, assuming
> > > 'iptables'");
> > > +backend = VIR_FIREWALLD_BACKEND_IPTABLES;
> > > +goto cleanup;
> > > +}
> > Surely we need an '} else {' case here to propagate 'error'
> > as fatal.
>
>
> The point is to ignore all errors. If error.level != VIR_ERR_ERROR, then
> there is no error to propagate (and if it is then we already ignored it). Am
> I missing something? (Very likely, since I can count on one hand the number
> of times I've had to directly interact with an error object.)
No, I'm just being dumb misreading this.
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 5/7] network: set firewalld zone of bridges to "libvirt" zone when appropriate
On 2/1/19 8:24 AM, Daniel P. Berrangé wrote: On Thu, Jan 31, 2019 at 08:24:56PM -0500, Laine Stump wrote: From: Laine Stump This patch restores broken guest network connectivity after a host firewalld is switched to using an nftables backend. It does this by adding libvirt networks' bridge interfaces to the new "libvirt" zone in firewalld. After this patch, the bridge interface of any network created by libvirt (when firewalld is active) will be added to the firewalld zone called "libvirt" if it exists (regardless of the firewalld backend setting). This behavior does *not* depend on whether or not libvirt has installed the libvirt zone file (set with "--with[out]-firewalld-zone" during the configure phase of the package build). If the libvirt zone doesn't exist (either because the package was configured to not install it, or possibly it was installed, but firewalld doesn't support rule priorities, resulting in a parse error), the bridge will remain in firewalld's default zone, which could be innocuous (in the case that the firewalld backend is iptables, guest networking will still function properly with the bridge in the default zone), or it could be disastrous (if the firewalld backend is nftables, we can be assured that guest networking will fail). In order to be unobtrusive in the former case, and informative in the latter, when the libvirt zone doesn't exist we then check the firewalld version to see if it's new enough to support the nftables backend, and then if the backend is actually set to nftables, before logging an error (and failing the net-start operation, since the network couldn't possibly work anyway). When the libvirt zone is used, network behavior is *slightly* different from behavior of previous libvirt. In the past, libvirt network behavior would be affected by the configuration of firewalld's default zone (usually "public"), but now it is affected only by the "libvirt" zone), and thus almost surely warrants a release note for any distro upgrading to libvirt 5.1 or above. Although it's unfortunate that we have to deal with a mandatory behavior change, the architecture of multiple hooks makes it impossible to *not* change behavior in some way, and the new behavior is arguably better (since it will now be possible to manage access to the host from virtual machines vs from public interfaces separately). Resolves: https://bugzilla.redhat.com/1638342 Creates-and-Resolves: https://bugzilla.redhat.com/1650320 Signed-off-by: Laine Stump --- NB: I had considered that it might be useful to cache the results of checking the list of active zones, the firewalld version, or the firewalld backend, but in my tests of restarting libvirtd with 100 active networks, the full startup time (from the beginning of "systemctl restart libvirtd.service" until successful return from a subsequent "virsh list") showed 42 seconds and a bit regardless of whether or not I made those checks. This tells me that the amount of time to be saved by caching the results of a single call vs calling once for each network are insignificant relative to everything else that is being done. Because any cached values would need to be stored in the network driver state object, and thus require acquiring the driver-wide lock to update at potentially very different times (e.g. in the response to a dbus message informing us that firewalld was restarted, as well as while starting a new network from an API call) I consider the chance of a bug in my code causing an obscure deadlock sometime in the future to be a much greater concern than maybe saving 1/10th of a second out of 42 (and lock contention might eliminate the gain anyway(), so I have left the code to retrieve the list of zones once for each network start. Changes in V2: * split off from Patch 5. This patch only sets the libvirt zone if it exists (and attempts to somewhat document the behavior in firewall.html), it doesn't install the libvirt zone. * check for existence of libvirt zone before attempting to set it. * if libvirt zone doesn't exist, only log an error in the case that the firewalld version is new enough to have an nftables backend, and the backend is actually set to nftables. docs/firewall.html.in | 33 + src/network/bridge_driver_linux.c | 48 +++ 2 files changed, 81 insertions(+) diff --git a/docs/firewall.html.in b/docs/firewall.html.in index 0a50687c26..5d584e582e 100644 --- a/docs/firewall.html.in +++ b/docs/firewall.html.in @@ -129,6 +129,39 @@ MASQUERADE all -- * * 192.168.122.0/24 !192.168.122.0/24 +firewalld and the virtual network driver + + + If https://firewalld.org;>firewalld is active on + the host, libvirt will attempt to place the bridge interface of + a libvirt virtual network into the firewalld zone named + "libvirt" (thus making all guest->host traffic on that network + subject to the
Re: [libvirt] [PATCH v2 3/7] util: new virFirewallD APIs + docs
On 2/1/19 8:17 AM, Daniel P. Berrangé wrote:
On Thu, Jan 31, 2019 at 08:24:54PM -0500, Laine Stump wrote:
+int
+virFirewallDGetBackend(void)
+{
+DBusConnection *sysbus = virDBusGetSystemBus();
+DBusMessage *reply = NULL;
+virError error;
+VIR_AUTOFREE(char *) backendStr = NULL;
+int backend = -1;
+
+if (!sysbus)
+return -1;
+
+memset(, 0, sizeof(error));
+
+if (virDBusCallMethod(sysbus,
+ ,
+ ,
+ VIR_FIREWALL_FIREWALLD_SERVICE,
+ "/org/fedoraproject/FirewallD1/config",
+ "org.freedesktop.DBus.Properties",
+ "Get",
+ "ss",
+ "org.fedoraproject.FirewallD1.config",
+ "FirewallBackend") < 0)
+goto cleanup;
+
+if (error.level == VIR_ERR_ERROR) {
+/* we don't want to log any error in the case that
+ * FirewallBackend isn't implemented in this firewalld, since
+ * that just means that it is an old version, and only has an
+ * iptables backend.
+ */
+VIR_DEBUG("Failed to get FirewallBackend setting, assuming
'iptables'");
+backend = VIR_FIREWALLD_BACKEND_IPTABLES;
+goto cleanup;
+}
Surely we need an '} else {' case here to propagate 'error'
as fatal.
The point is to ignore all errors. If error.level != VIR_ERR_ERROR, then
there is no error to propagate (and if it is then we already ignored
it). Am I missing something? (Very likely, since I can count on one hand
the number of times I've had to directly interact with an error object.)
+
+if (virDBusMessageRead(reply, "v", "s", ) < 0)
+goto cleanup;
+
+VIR_DEBUG("FirewallD backend: %s", backendStr);
+
+if ((backend = virFirewallDBackendTypeFromString(backendStr)) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Unrecognized firewalld backend type: %s"),
+ backendStr);
+}
I'd usually put an explicit 'goto cleanup' here. I've seen bugs
in the past where people optimized by assuming we'll immediately
hit the cleanup, but someome later introduces more code and
doesn't notice the missing goto.
Yeah, me too. I just missed it. I'll add it in.
+
+ cleanup:
+virResetError();
+virDBusMessageUnref(reply);
+return backend;
+}
+/**
+ * virFirewallDGetZones:
+ * @zones: array of char *, each entry is a null-terminated zone name
+ * @nzones: number of entries in @zones
+ *
+ * Get the number of currently active firewalld zones, and their names in an
+ * array of null-terminated strings.
+ *
+ * Returns 0 on success, -1 (and failure logged) on error
+ */
+int
+virFirewallDGetZones(char ***zones, size_t *nzones)
+{
+DBusConnection *sysbus = virDBusGetSystemBus();
+DBusMessage *reply = NULL;
+int ret = -1;
+
+*nzones = 0;
+*zones = NULL;
+
+if (!sysbus)
+return -1;
+
+if (virDBusCallMethod(sysbus,
+ ,
+ NULL,
+ VIR_FIREWALL_FIREWALLD_SERVICE,
+ "/org/fedoraproject/FirewallD1",
+ "org.fedoraproject.FirewallD1.zone",
+ "getZones",
+ NULL) < 0)
+goto cleanup;
+
+if (virDBusMessageRead(reply, "a", nzones, zones) < 0)
+goto cleanup;
+
+ret = 0;
+ cleanup:
+virDBusMessageUnref(reply);
+return ret;
+}
+
+
+/**
+ * virFirewallDZoneExists:
+ * @match: name of zone to look for
+ *
+ * Returns true if the requested zone exists, or false if it doesn't exist
+ */
+bool
+virFirewallDZoneExists(const char *match)
+{
+size_t nzones = 0, i;
+char **zones = NULL;
+bool result = false;
+
+return true;
+
+if (virFirewallDGetZones(, ) < 0)
+goto cleanup;
+
+for (i = 0; i < nzones; i++) {
+VIR_DEBUG("FirewallD zone: %s", zones[i]);
+if (STREQ_NULLABLE(zones[i], match))
+result = true;
+}
+
+ cleanup:
+/* NB: zones points to memory inside reply, so it is cleaned
+ * up by virDBusMessageUnref, and doesn't need to be freed
+ */
I'm confused by this comment. virDBusMessageUnref has already
run before control even returned to this method, so it 'zones'
is owned by the reply, we've been using free'd memory.
The very next lines, however, contradict this comment by
freein'g zones anyway, which makes me even more confused :-)
Yeah, I changed the code and forgot to remove the comment (even through
probably a dozen or more rebases *and* moving a large chunk of this
function into the preceding virFirewalldGetZones() function!! Somehow my
brain just glossed right over the comment as I was changing and moving
the code.)
The comment arose from me misunderstanding something you said in IRC (or
maybe you misunderstood what I was asking
Re: [libvirt] [PATCH v2 3/4] util: Introduce virStorageFileGetNPIVKey
On Tue, Jan 29, 2019 at 03:54:42PM +0100, Michal Privoznik wrote:
On 1/18/19 3:42 PM, John Ferlan wrote:
The vHBA/NPIV LUNs created via the udev processing of the
VPORT_CREATE command end up using the same serial value
as seen/generated by the /lib/udev/scsi_id as returned
during virStorageFileGetSCSIKey. Therefore, in order to
generate a unique enough key to be used when adding the
LUN as a volume during virStoragePoolObjAddVol a more
unique key needs to be generated for an NPIV volume.
The problem is illustrated by the following example, where
scsi_host5 is a vHBA used with the following LUNs:
$ lsscsi -tg
...
[5:0:4:0]diskfc:0x5006016844602198,0x101f00 /dev/sdh /dev/sg23
[5:0:5:0]diskfc:0x5006016044602198,0x102000 /dev/sdi /dev/sg24
...
Calling virStorageFileGetSCSIKey would return:
/lib/udev/scsi_id --device /dev/sdh --whitelisted --replace-whitespace /dev/sdh
350060160c460219850060160c4602198
/lib/udev/scsi_id --device /dev/sdh --whitelisted --replace-whitespace /dev/sdi
350060160c460219850060160c4602198
Note that althrough /dev/sdh and /dev/sdi are separate LUNs, they
end up with the same serial number used for the vol->key value.
When virStoragePoolFCRefreshThread calls virStoragePoolObjAddVol
the second LUN fails to be added with the following message
getting logged:
virHashAddOrUpdateEntry:341 : internal error: Duplicate key
To resolve this, virStorageFileGetNPIVKey will use a similar call
sequence as virStorageFileGetSCSIKey, except that it will add the
"--export" option to the call. This results in more detailed output
which needs to be parsed in order to formulate a unique enough key
to be used. In order to be unique enough, the returned value will
concatenate the target port as returned in the "ID_TARGET_PORT"
field from the command to the "ID_SERIAL" value.
Signed-off-by: John Ferlan
---
src/libvirt_private.syms | 1 +
src/util/virstoragefile.c | 80 +++
src/util/virstoragefile.h | 2 +
3 files changed, 83 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c3d6306809..bdc2877a9f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2861,6 +2861,7 @@ virStorageFileGetMetadata;
virStorageFileGetMetadataFromBuf;
virStorageFileGetMetadataFromFD;
virStorageFileGetMetadataInternal;
+virStorageFileGetNPIVKey;
virStorageFileGetRelativeBackingPath;
virStorageFileGetSCSIKey;
virStorageFileGetUniqueIdentifier;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 2511511d14..759d0625b6 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1490,6 +1490,86 @@ int virStorageFileGetSCSIKey(const char *path,
#endif
+#ifdef WITH_UDEV
+/* virStorageFileGetNPIVKey
+ * @path: Path to the NPIV device
+ * @key: Unique key to be returned
+ *
+ * Using a udev specific function, query the @path to get and return a
+ * unique @key for the caller to use. Unlike the GetSCSIKey method, an
+ * NPIV LUN is uniquely identified by it's ID_TARGET_PORT value.
*its
+ *
+ * Returns:
+ * 0 On success, with the @key filled in or @key=NULL if the
+ * returned output string didn't have the data we need to
+ * formulate a unique key value
[0]
+ *-1 When WITH_UDEV is undefined and a system error is reported
+ *-2 When WITH_UDEV is defined, but calling virCommandRun fails
+ */
+int
+virStorageFileGetNPIVKey(const char *path,
+ char **key)
+{
+int status;
+VIR_AUTOFREE(char *) outbuf = NULL;
+const char *serial;
+const char *port;
+virCommandPtr cmd = virCommandNewArgList("/lib/udev/scsi_id",
+ "--replace-whitespace",
+ "--whitelisted",
+ "--export",
+ "--device", path,
+ NULL
+ );
+int ret = -2;
+
+*key = NULL;
+
+/* Run the program and capture its output */
+virCommandSetOutputBuffer(cmd, );
+if (virCommandRun(cmd, ) < 0)
+goto cleanup;
+
+/* Explicitly check status == 0, rather than passing NULL
+ * to virCommandRun because we don't want to raise an actual
+ * error in this scenario, just return a NULL key.
+ */
+if (status == 0 && *outbuf &&
+(serial = strstr(outbuf, "ID_SERIAL=")) &&
+(port = strstr(outbuf, "ID_TARGET_PORT="))) {
+char *serial_eq = strchr(serial, '=');
+char *serial_nl = strchr(serial, '\n');
+char *port_eq = strchr(port, '=');
+char *port_nl = strchr(port, '\n');
+
+if (serial_eq)
+serial = serial_eq + 1;
+if (serial_nl)
+*serial_nl = '\0';
+if (port_eq)
+port = port_eq + 1;
+if (port_nl)
+*port_nl = '\0';
For my one SCSI device,
Re: [libvirt] [PATCH v2 2/4] storage: Rework virStorageBackendSCSISerial
On Wed, Jan 30, 2019 at 11:55:10AM -0500, John Ferlan wrote:
On 1/29/19 10:14 AM, Ján Tomko wrote:
On Fri, Jan 18, 2019 at 09:42:35AM -0500, John Ferlan wrote:
Alter the code to use the virStorageFileGetSCSIKey helper
to fetch the unique key for the SCSI disk. Alter the logic
to follow the former code which would return a duplicate
of @dev when either the virCommandRun succeeded, but returned
an empty string or when WITH_UDEV was not true.
Signed-off-by: John Ferlan
---
src/storage/storage_util.c | 34 --
1 file changed, 8 insertions(+), 26 deletions(-)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index a84ee5b600..aa1af434de 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -3758,36 +3758,18 @@
virStorageBackendRefreshLocal(virStoragePoolObjPtr pool)
static char *
virStorageBackendSCSISerial(const char *dev)
{
+ int rc;
char *serial = NULL;
-#ifdef WITH_UDEV
- virCommandPtr cmd = virCommandNewArgList(
- "/lib/udev/scsi_id",
- "--replace-whitespace",
- "--whitelisted",
- "--device", dev,
- NULL
- );
-
- /* Run the program and capture its output */
- virCommandSetOutputBuffer(cmd, );
- if (virCommandRun(cmd, NULL) < 0)
- goto cleanup;
-#endif
- if (serial && STRNEQ(serial, "")) {
- char *nl = strchr(serial, '\n');
- if (nl)
- *nl = '\0';
- } else {
- VIR_FREE(serial);
- ignore_value(VIR_STRDUP(serial, dev));
- }
+ rc = virStorageFileGetSCSIKey(dev, );
+ if (rc == 0 && serial)
+ return serial;
-#ifdef WITH_UDEV
- cleanup:
- virCommandFree(cmd);
-#endif
+ if (rc == -2)
+ return NULL;
+ virResetLastError();
Every virReportError call logs the error into the configured log outputs
and sets the thread-local error object.
This only resets the error object, there's no way to unlog the error.
If it's expected operation, we should not log an error in the first
place.
Jano
So does the attached resolve the concern/issue you have?
Tks,
John
From 6f9385248a9e78cc3619d02e4adf3205cbad874d Mon Sep 17 00:00:00 2001
From: John Ferlan
Date: Tue, 29 Jan 2019 16:44:01 -0500
Subject: [PATCH] Squash with previous
Signed-off-by: John Ferlan
---
src/locking/lock_driver_lockd.c | 2 +-
src/storage/storage_util.c | 3 +--
src/util/virstoragefile.c | 10 +++---
src/util/virstoragefile.h | 3 ++-
4 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
index 16fce551c3..4ffa92fc9b 100644
--- a/src/locking/lock_driver_lockd.c
+++ b/src/locking/lock_driver_lockd.c
@@ -531,7 +531,7 @@ static int
virLockManagerLockDaemonAddResource(virLockManagerPtr lock,
if (STRPREFIX(name, "/dev") &&
driver->scsiLockSpaceDir) {
VIR_DEBUG("Trying to find an SCSI ID for %s", name);
-if (virStorageFileGetSCSIKey(name, ) < 0)
+if (virStorageFileGetSCSIKey(name, , false) < 0)
goto cleanup;
if (newName) {
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index d8fd76f6b6..85f1cbb57d 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -3782,14 +3782,13 @@ virStorageBackendSCSISerial(const char *dev)
int rc;
char *serial = NULL;
-rc = virStorageFileGetSCSIKey(dev, );
+rc = virStorageFileGetSCSIKey(dev, , true);
if (rc == 0 && serial)
return serial;
if (rc == -2)
return NULL;
-virResetLastError();
ignore_value(VIR_STRDUP(serial, dev));
return serial;
}
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 2a38a08241..39c8511bef 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1429,6 +1429,7 @@ int virStorageFileGetLVMKey(const char *path,
/* virStorageFileGetSCSIKey
* @path: Path to the SCSI device
* @key: Unique key to be returned
+ * @ignoreError: Used to not report ENOSYS
*
* Using a udev specific function, query the @path to get and return a
* unique @key for the caller to use.
@@ -1441,7 +1442,8 @@ int virStorageFileGetLVMKey(const char *path,
*/
int
virStorageFileGetSCSIKey(const char *path,
- char **key)
+ char **key,
+ bool ignoreError ATTRIBUTE_UNUSED)
{
int status;
virCommandPtr cmd = virCommandNewArgList("/lib/udev/scsi_id",
@@ -1481,9 +1483,11 @@ virStorageFileGetSCSIKey(const char *path,
}
#else
int virStorageFileGetSCSIKey(const char *path,
- char **key ATTRIBUTE_UNUSED)
+ char **key ATTRIBUTE_UNUSED,
+ bool ignoreError)
{
-virReportSystemError(ENOSYS, _("Unable to get SCSI key for %s"), path);
+if (!ignoreError)
+virReportSystemError(ENOSYS, _("Unable to get SCSI key for %s"), path);
return -1;
}
Re: [libvirt] [PATCH v2 1/4] util: Modify virStorageFileGetSCSIKey return
On Fri, Jan 18, 2019 at 09:42:34AM -0500, John Ferlan wrote:
Alter the "real" code to return -2 on virCommandRun failure.
Alter the comments and function header to describe the function
and it's returns.
*its
Signed-off-by: John Ferlan
---
src/util/virstoragefile.c | 21 ++---
1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index bd4b0274df..2511511d14 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1425,9 +1425,24 @@ int virStorageFileGetLVMKey(const char *path,
}
#endif
+
Unrelated whitespace change
#ifdef WITH_UDEV
-int virStorageFileGetSCSIKey(const char *path,
- char **key)
+/* virStorageFileGetSCSIKey
+ * @path: Path to the SCSI device
+ * @key: Unique key to be returned
+ *
+ * Using a udev specific function, query the @path to get and return a
+ * unique @key for the caller to use.
+ *
+ * Returns:
+ * 0 On success, with the @key filled in or @key=NULL if the
+ * returned string was empty.
+ *-1 When WITH_UDEV is undefined and a system error is reported
+ *-2 When WITH_UDEV is defined, but calling virCommandRun fails
+ */
+int
+virStorageFileGetSCSIKey(const char *path,
+ char **key)
{
int status;
virCommandPtr cmd = virCommandNewArgList("/lib/udev/scsi_id",
Reviewed-by: Ján Tomko
Jano
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 4/7] configure: selectively install a firewalld 'libvirt' zone
On 2/1/19 8:28 AM, Eric Garver wrote: On Thu, Jan 31, 2019 at 10:10:43PM -0500, Laine Stump wrote: On 1/31/19 8:24 PM, Laine Stump wrote: Changes from V1: [...] * make the rule's priority 32767 instead of 127. [...] + + + + I found out after sending this that when I make the priority of the reject rule 32767 instead of 127, it's apparently ignored (in my example, I was able to ssh to port 222 of the host even though the zone doesn't allow that). Eric, any idea why this might be happening? What build are you testing against? At one point the limit was 127, but I increased it before pushing it upstream. You can check the firewalld logs - there may be an error reporting the above priority is out of range. Ah, maybe you haven't backported that change to RHEL? I was testing on my RHEL8 beta system. If that's the case, then either we need that change backported to RHEL too, or I need to change the priority back to 127. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 6/7] network: allow configuring firewalld zone for virtual network bridge device
On Thu, Jan 31, 2019 at 08:24:57PM -0500, Laine Stump wrote: > Since we're setting the zone anyway, it will be useful to allow > setting a different (custom) zone for each network. This will be done > by adding a "zone" attribute to the "bridge" element, e.g.: > >... > >... > > If a zone is specified in the config and it can't be honored, this > will be an error. > > Signed-off-by: Laine Stump > --- > > Change from V1: move news.xml additions to a separate patch, as requested. > > docs/firewall.html.in | 5 + > docs/formatnetwork.html.in | 17 + > docs/schemas/basictypes.rng| 6 ++ > docs/schemas/network.rng | 6 ++ > src/conf/network_conf.c| 14 -- > src/conf/network_conf.h| 1 + > src/network/bridge_driver_linux.c | 19 +++ > tests/networkxml2xmlin/routed-network.xml | 2 +- > tests/networkxml2xmlout/routed-network.xml | 2 +- > 9 files changed, 68 insertions(+), 4 deletions(-) Reviewed-by: Daniel P. Berrangé 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 7/7] docs: update news.xml for firewalld zone changes
On Thu, Jan 31, 2019 at 08:24:58PM -0500, Laine Stump wrote: > Signed-off-by: Laine Stump > --- > > New in V2. Split off from previous patch. > > docs/news.xml | 40 > 1 file changed, 40 insertions(+) Reviewed-by: Daniel P. Berrangé 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 4/7] configure: selectively install a firewalld 'libvirt' zone
On Thu, Jan 31, 2019 at 10:10:43PM -0500, Laine Stump wrote: > On 1/31/19 8:24 PM, Laine Stump wrote: > > Changes from V1: > > [...] > > > * make the rule's priority 32767 instead of 127. > > [...] > > > + > > + > > + > > + > > > I found out after sending this that when I make the priority of the reject > rule 32767 instead of 127, it's apparently ignored (in my example, I was > able to ssh to port 222 of the host even though the zone doesn't allow > that). Some kind of boundary condition i guess. Perhaps 32766 will work ? > > > Eric, any idea why this might be happening? > > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list 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 4/7] configure: selectively install a firewalld 'libvirt' zone
On Thu, Jan 31, 2019 at 10:10:43PM -0500, Laine Stump wrote: > On 1/31/19 8:24 PM, Laine Stump wrote: > > Changes from V1: > > [...] > > > * make the rule's priority 32767 instead of 127. > > [...] > > > + > > + > > + > > + > > > I found out after sending this that when I make the priority of the reject > rule 32767 instead of 127, it's apparently ignored (in my example, I was > able to ssh to port 222 of the host even though the zone doesn't allow > that). > > > Eric, any idea why this might be happening? What build are you testing against? At one point the limit was 127, but I increased it before pushing it upstream. You can check the firewalld logs - there may be an error reporting the above priority is out of range. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 5/7] network: set firewalld zone of bridges to "libvirt" zone when appropriate
On Thu, Jan 31, 2019 at 08:24:56PM -0500, Laine Stump wrote: > From: Laine Stump > > This patch restores broken guest network connectivity after a host > firewalld is switched to using an nftables backend. It does this by > adding libvirt networks' bridge interfaces to the new "libvirt" zone > in firewalld. > > After this patch, the bridge interface of any network created by > libvirt (when firewalld is active) will be added to the firewalld > zone called "libvirt" if it exists (regardless of the firewalld > backend setting). This behavior does *not* depend on whether or not > libvirt has installed the libvirt zone file (set with > "--with[out]-firewalld-zone" during the configure phase of the package > build). > > If the libvirt zone doesn't exist (either because the package was > configured to not install it, or possibly it was installed, but > firewalld doesn't support rule priorities, resulting in a parse > error), the bridge will remain in firewalld's default zone, which > could be innocuous (in the case that the firewalld backend is > iptables, guest networking will still function properly with the > bridge in the default zone), or it could be disastrous (if the > firewalld backend is nftables, we can be assured that guest networking > will fail). In order to be unobtrusive in the former case, and > informative in the latter, when the libvirt zone doesn't exist we > then check the firewalld version to see if it's new enough to support > the nftables backend, and then if the backend is actually set to > nftables, before logging an error (and failing the net-start > operation, since the network couldn't possibly work anyway). > > When the libvirt zone is used, network behavior is *slightly* > different from behavior of previous libvirt. In the past, libvirt > network behavior would be affected by the configuration of firewalld's > default zone (usually "public"), but now it is affected only by the > "libvirt" zone), and thus almost surely warrants a release note for > any distro upgrading to libvirt 5.1 or above. Although it's > unfortunate that we have to deal with a mandatory behavior change, the > architecture of multiple hooks makes it impossible to *not* change > behavior in some way, and the new behavior is arguably better (since > it will now be possible to manage access to the host from virtual > machines vs from public interfaces separately). > > Resolves: https://bugzilla.redhat.com/1638342 > Creates-and-Resolves: https://bugzilla.redhat.com/1650320 > Signed-off-by: Laine Stump > --- > > NB: I had considered that it might be useful to cache the results of > checking the list of active zones, the firewalld version, or the > firewalld backend, but in my tests of restarting libvirtd with 100 > active networks, the full startup time (from the beginning of > "systemctl restart libvirtd.service" until successful return from a > subsequent "virsh list") showed 42 seconds and a bit regardless of > whether or not I made those checks. This tells me that the amount of > time to be saved by caching the results of a single call vs calling > once for each network are insignificant relative to everything else > that is being done. Because any cached values would need to be stored > in the network driver state object, and thus require acquiring the > driver-wide lock to update at potentially very different times > (e.g. in the response to a dbus message informing us that firewalld > was restarted, as well as while starting a new network from an API > call) I consider the chance of a bug in my code causing an obscure > deadlock sometime in the future to be a much greater concern than > maybe saving 1/10th of a second out of 42 (and lock contention might > eliminate the gain anyway(), so I have left the code to retrieve the > list of zones once for each network start. > > Changes in V2: > > * split off from Patch 5. This patch only sets the libvirt zone if it > exists (and attempts to somewhat document the behavior in > firewall.html), it doesn't install the libvirt zone. > > * check for existence of libvirt zone before attempting to set it. > > * if libvirt zone doesn't exist, only log an error in the case that > the firewalld version is new enough to have an nftables backend, and > the backend is actually set to nftables. > > > docs/firewall.html.in | 33 + > src/network/bridge_driver_linux.c | 48 +++ > 2 files changed, 81 insertions(+) > > diff --git a/docs/firewall.html.in b/docs/firewall.html.in > index 0a50687c26..5d584e582e 100644 > --- a/docs/firewall.html.in > +++ b/docs/firewall.html.in > @@ -129,6 +129,39 @@ MASQUERADE all -- * * 192.168.122.0/24 > !192.168.122.0/24 > > > > +firewalld and the > virtual network driver > + > + > + If https://firewalld.org;>firewalld is active on > + the host, libvirt will attempt to place the bridge interface of > + a
Re: [libvirt] [PATCH v2 4/7] configure: selectively install a firewalld 'libvirt' zone
On Thu, Jan 31, 2019 at 08:24:55PM -0500, Laine Stump wrote: > In the past (when both libvirt and firewalld used iptables), if either > libvirt's rules *OR* firewalld's rules accepted a packet, it would > be accepted. This was because libvirt and firewalld rules were > processed during the same kernel hook, and a single ACCEPT result > would terminate the rule traversal and cause the packet to be > accepted. > > But now firewalld can use nftables for its backend, while libvirt's > firewall rules are still using iptables; iptables rules are still > processed, but at a different time during packet processing > (i.e. during a different hook) than the firewalld nftables rules. The > result is that a packet must be accepted by *BOTH* the libvirt > iptables rules *AND* the firewalld nftable rules in order to be > accepted. > > This causes pain because > > 1) libvirt always adds rules to permit DNS and DHCP (and sometimes > TFTP) from guests to the host network's bridge interface. But > libvirt's bridges are in firewalld's "default" zone (which is usually > the zone called "public"). The public zone allows ssh, but doesn't > allow DNS, DHCP, or TFTP. So even though libvirt's rules allow the > DHCP and DNS traffic, the firewalld rules (now processed during a > different hook) dont, thus guests connected to libvirt's bridges can't > acquire an IP address from DHCP, nor can they make DNS queries to the > DNS server libvirt has setup on the host. (This could be solved by > modifying the default firewalld zone to allow DNS and DHCP, but that > would open *all* interfaces in the default zone to those services, > which is most likely not what the host's admin wants.) > > 2) Even though libvirt adds iptables rules to allow forwarded traffic > to pass the iptables hook, firewalld's higher level "rich rules" don't > yet have the ability to configure the acceptance of forwarded traffic > (traffic that is going somewhere beyond the host), so any traffic that > needs to be forwarded from guests to the network beyond the host is > rejected during the nftables hook by the default zone's "default > reject" policy (which rejects all traffic in the zone not specifically > allowed by the rules in the zone, whether that traffic is destined to > be forwarded or locally received by the host). > > libvirt can't send "direct" nftables rules (firewalld only supports > direct/passthrough rules for iptables), so we can't solve this problem > by just sending explicit nftables rules instead of explicit iptables > rules (which, if it could be done, would place libvirt's rules in the > same hook as firewalld's native rules, and thus eliminate the need for > packets to be accepted by both libvirt's and firewalld's own rules). > > However, we can take advantage of a quirk in firewalld zones that have > a default policy of "accept" (meaning any packet that doesn't match a > specific rule in the zone will be *accepted*) - this default accept will > also accept forwarded traffic (not just traffic destined for the host). > > Of course we don't want to modify firewalld's default zone in that > way, because that would affect the filtering of traffic coming into > the host from other interfaces using that zone. Instead, we will > create a new zone called "libvirt". The libvirt zone will have a > default policy of accept so that forwarded traffic can pass andn list s/andn/and/ > specific services that will be allowed into the host from guests (DNS, > DHCP, SSH, and TFTP). Reviewed-by: Daniel P. Berrangé 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 3/7] util: new virFirewallD APIs + docs
On Thu, Jan 31, 2019 at 08:24:54PM -0500, Laine Stump wrote:
> virFirewallDGetBackend() reports whether firewalld is currently using
> an iptables or an nftables backend.
>
> virFirewallDGetVersion() learns the version of the firewalld running
> on this system and returns it as 100*major + 1000*minor + micro.
>
> virFirewallDGetZones() gets a list of all currently active firewalld
> zones.
>
> virFirewallDInterfaceSetZone() sets the firewalld zone of the given
> interface.
>
> virFirewallDZoneExists() can be used to learn whether or not a
> particular zone is present and active in firewalld.
>
> Signed-off-by: Laine Stump
> ---
> +int
> +virFirewallDGetBackend(void)
> +{
> +DBusConnection *sysbus = virDBusGetSystemBus();
> +DBusMessage *reply = NULL;
> +virError error;
> +VIR_AUTOFREE(char *) backendStr = NULL;
> +int backend = -1;
> +
> +if (!sysbus)
> +return -1;
> +
> +memset(, 0, sizeof(error));
> +
> +if (virDBusCallMethod(sysbus,
> + ,
> + ,
> + VIR_FIREWALL_FIREWALLD_SERVICE,
> + "/org/fedoraproject/FirewallD1/config",
> + "org.freedesktop.DBus.Properties",
> + "Get",
> + "ss",
> + "org.fedoraproject.FirewallD1.config",
> + "FirewallBackend") < 0)
> +goto cleanup;
> +
> +if (error.level == VIR_ERR_ERROR) {
> +/* we don't want to log any error in the case that
> + * FirewallBackend isn't implemented in this firewalld, since
> + * that just means that it is an old version, and only has an
> + * iptables backend.
> + */
> +VIR_DEBUG("Failed to get FirewallBackend setting, assuming
> 'iptables'");
> +backend = VIR_FIREWALLD_BACKEND_IPTABLES;
> +goto cleanup;
> +}
Surely we need an '} else {' case here to propagate 'error'
as fatal.
> +
> +if (virDBusMessageRead(reply, "v", "s", ) < 0)
> +goto cleanup;
> +
> +VIR_DEBUG("FirewallD backend: %s", backendStr);
> +
> +if ((backend = virFirewallDBackendTypeFromString(backendStr)) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Unrecognized firewalld backend type: %s"),
> + backendStr);
> +}
I'd usually put an explicit 'goto cleanup' here. I've seen bugs
in the past where people optimized by assuming we'll immediately
hit the cleanup, but someome later introduces more code and
doesn't notice the missing goto.
> +
> + cleanup:
> +virResetError();
> +virDBusMessageUnref(reply);
> +return backend;
> +}
> +/**
> + * virFirewallDGetZones:
> + * @zones: array of char *, each entry is a null-terminated zone name
> + * @nzones: number of entries in @zones
> + *
> + * Get the number of currently active firewalld zones, and their names in an
> + * array of null-terminated strings.
> + *
> + * Returns 0 on success, -1 (and failure logged) on error
> + */
> +int
> +virFirewallDGetZones(char ***zones, size_t *nzones)
> +{
> +DBusConnection *sysbus = virDBusGetSystemBus();
> +DBusMessage *reply = NULL;
> +int ret = -1;
> +
> +*nzones = 0;
> +*zones = NULL;
> +
> +if (!sysbus)
> +return -1;
> +
> +if (virDBusCallMethod(sysbus,
> + ,
> + NULL,
> + VIR_FIREWALL_FIREWALLD_SERVICE,
> + "/org/fedoraproject/FirewallD1",
> + "org.fedoraproject.FirewallD1.zone",
> + "getZones",
> + NULL) < 0)
> +goto cleanup;
> +
> +if (virDBusMessageRead(reply, "a", nzones, zones) < 0)
> +goto cleanup;
> +
> +ret = 0;
> + cleanup:
> +virDBusMessageUnref(reply);
> +return ret;
> +}
> +
> +
> +/**
> + * virFirewallDZoneExists:
> + * @match: name of zone to look for
> + *
> + * Returns true if the requested zone exists, or false if it doesn't exist
> + */
> +bool
> +virFirewallDZoneExists(const char *match)
> +{
> +size_t nzones = 0, i;
> +char **zones = NULL;
> +bool result = false;
> +
> +return true;
> +
> +if (virFirewallDGetZones(, ) < 0)
> +goto cleanup;
> +
> +for (i = 0; i < nzones; i++) {
> +VIR_DEBUG("FirewallD zone: %s", zones[i]);
> +if (STREQ_NULLABLE(zones[i], match))
> +result = true;
> +}
> +
> + cleanup:
> +/* NB: zones points to memory inside reply, so it is cleaned
> + * up by virDBusMessageUnref, and doesn't need to be freed
> + */
I'm confused by this comment. virDBusMessageUnref has already
run before control even returned to this method, so it 'zones'
is owned by the reply, we've been using free'd memory.
The very next lines, however, contradict this comment by
freein'g zones anyway, which makes me even more
Re: [libvirt] [PATCH v2 2/7] util: move all firewalld-specific stuff into its own files
On Thu, Jan 31, 2019 at 08:24:53PM -0500, Laine Stump wrote: > In preparation for adding several other firewalld-specific functions, > separate the code that's unique to firewalld from the more-generic > "firewall" file. > > Signed-off-by: Laine Stump > --- > > Change from V1: define VIR_FIREWALL_FIREWALLD_SERVICE in virfirewalldpriv.h, > since it should only be used by virfirewalld.c and unit test programs. > > (no, I haven't done anything about the awkward error checking that was > already existing in the code I moved - not only is that a separate > issue, but there no existing way to fix it anyway - that would take > changes to firewalld) > > include/libvirt/virterror.h | 1 + > src/libvirt_private.syms| 5 ++ > src/util/Makefile.inc.am| 3 + > src/util/virerror.c | 3 +- > src/util/virfirewall.c | 86 +--- > src/util/virfirewalld.c | 151 > src/util/virfirewalld.h | 33 > src/util/virfirewalldpriv.h | 30 +++ > src/util/virfirewallpriv.h | 2 - > tests/virfirewalltest.c | 2 + > 10 files changed, 231 insertions(+), 85 deletions(-) > create mode 100644 src/util/virfirewalld.c > create mode 100644 src/util/virfirewalld.h > create mode 100644 src/util/virfirewalldpriv.h Reviewed-by: Daniel P. Berrangé 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 1/7] configure: change HAVE_FIREWALLD to WITH_FIREWALLD
On Thu, Jan 31, 2019 at 08:24:52PM -0500, Laine Stump wrote: > Support for firewalld is a feature that can be selectively enabled or > disabled (using --with-firewalld/--without-firewalld), not merely > something that must be accounted for in the code if it is present with > no exceptions. It is more consistent with other usage in libvirt to > use WITH_FIREWALLD rather than HAVE_FIREWALLD. > > Signed-off-by: Laine Stump > --- > > New patch in V2 (NB, I already pushed patch 1 from V1, as it was > ACKed, and not directly related to the rest of the series) > > m4/virt-firewalld.m4 | 4 ++-- > src/network/bridge_driver.c| 6 +++--- > src/nwfilter/nwfilter_driver.c | 6 +++--- > 3 files changed, 8 insertions(+), 8 deletions(-) Reviewed-by: Daniel P. Berrangé 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] [Qemu-devel] Configuring pflash devices for OVMF firmware
>> This is all greek to me. I take it there's something wrong with these >> accelerators that makes (read-only?) flash memory not work, even >> though the read-only mapping we now create for traditional BIOS works. >> Weird, but I'm of course willing to take your word for it. > Yes, as I wrote in the other message even read-only flash memory supports > commands, and these accelerators do not support direct reads + MMIO writes on > the same memory slot. > At least I checked HAX code and it doesn't; I don't know about WHPX. (CC'd Yu Ning @ Intel's HAXM team) Not sure, if I'm understanding the issue correctly, but isn't `HAX_VM_IOCTL_SET_RAM2` with the `HAX_RAM_INFO_ROM` flag precisely what you are looking for? More precisely, HAX_VM_IOCTL_SET_RAM2 maps an HVA range to a GPA range, the HAX_RAM_INFO_ROM flag should allow only guest memory reads to that range [1]. When the guest attempts to write, this should trigger a VM exit that will be handled by QEMU. Also, this seems to be handled here: https://github.com/qemu/qemu/blob/15bede554162dda822cd762c689edb6fa32b6e3b/target/i386/hax-mem.c#L205-L207 Best, Alexandro [1] https://github.com/intel/haxm/blob/master/docs/api.md -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] Configuring pflash devices for OVMF firmware
> -Original Message-
> From: Paolo Bonzini [mailto:pbonz...@redhat.com]
> Sent: Friday, February 1, 2019 7:55
> To: Alexandro Sanchez Bach ; 'Markus Armbruster'
>
> Cc: 'Peter Maydell' ; 'Peter Krempa'
> ; 'Qemu-block' ; 'Libvirt'
> ; 'QEMU Developers' ;
> 'László Érsek' ; 'Justin Terry (VM)'
> ; Ning, Yu
> Subject: Re: [Qemu-devel] Configuring pflash devices for OVMF firmware
>
> On 01/02/19 00:28, Alexandro Sanchez Bach wrote:
> > (CC'd Yu Ning @ Intel's HAXM team)
> >
> > Not sure, if I'm understanding the issue correctly, but isn't
> > `HAX_VM_IOCTL_SET_RAM2` with the `HAX_RAM_INFO_ROM` flag precisely
> > what you are looking for?
> >
> > More precisely, HAX_VM_IOCTL_SET_RAM2 maps an HVA range to a GPA
> > range, the HAX_RAM_INFO_ROM flag should allow only guest memory reads
> > to that range [1]. When the guest attempts to write, this should
> > trigger a VM exit that will be handled by QEMU.
>
> The missing handling is in the hypervisor:
>
> if (ret == -EACCES) {
> /*
> * For some reason, during boot-up, Chrome OS guests make
> hundreds of
> * attempts to write to GPAs close to 4GB, which are mapped into
> BIOS
> * (read-only) and thus result in EPT violations.
> * TODO: Handle this case properly.
> */
> hax_warning("%s: Unexpected EPT violation cause. Skipping
> instruction"
> " (len=%u)\n", __func__,
> vcpu->vmx.exit_instr_length);
> advance_rip(vcpu);
> return HAX_EXIT;
> }
>
> > Also, this seems to be handled here:
> >
> https://github.com/qemu/qemu/blob/15bede554162dda822cd762c689edb6fa3
> 2b
> > 6e3b/target/i386/hax-mem.c#L205-L207
>
> Right, though to be precise it should be changed to
>
> if (memory_region_is_rom(section->mr) ||
>memory_region_is_romd(section->mr)) { flags |=
> HAX_RAM_INFO_ROM;
> }
>
> for that to work.
>
Thank you both for outlining the changes we have to make in order to support
ROMD memory regions! The only question is whether we should pass a new flag to
HAX_VM_IOCTL_SET_RAM2 for ROMD, so the hypervisor could respond differently to
writes to ROM and ROMD regions. Would that be useful at all? What would
happen if HAXM asked QEMU to emulate a write to ROM?
HAXM didn't implement ROMD support at first, because the guests we tested could
boot without it (including Chrome OS). Now that this feature has become more
popular (and we want to be able to boot OVMF), I think it's time to get it
done. I'd like to get to it after the Lunar New Year holidays, but if anyone
can finish it sooner, I'll be happy to merge their patch into HAXM.
-Yu
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: news: Update the release notes with the SEV permission fix
On Fri, 2019-02-01 at 13:07 +0100, Erik Skultety wrote: [...] > + By default, libvirt runs the QEMU process as qemu:qemu which could > + cause issues during probing as some features (like AMD SEV) might > be > + inaccesible to QEMU because of file system permissions. Therefore, > + CAP_DAC_OVERRIDE is granted to overcome these for the purposes of > + probing. You can wrap 'qemu:qemu' and 'CAP_DAC_OVERRIDE' in elements so that they will look nicer in the HTML version. I'd also change ' (like AMD SEV)' to ', like AMD SEV,', but feel free to leave it alone if you like it better that way. Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] scsi-disk: Device Identification fixes
Am 25.01.2019 um 18:46 hat Kevin Wolf geschrieben: > The vendor specific designator in the Device Identification VPD page has > two problems: > > 1. It defaults to the BlockBackend name (-drive id=...), which everyone >expected to be a host detail that the guest never sees > > 2. With -blockdev based setups it defaults to an empty string; if this >default is used with more than one disk, the guest OS will interpret >this as a single multipath disk. > > We can address problem 2 immediately, and start running the deprecation > clock for problem 1. > > Related bug reports: > https://bugzilla.redhat.com/show_bug.cgi?id=1669446 > https://bugzilla.redhat.com/show_bug.cgi?id=1668248 Applied patches 1 and 2 to the block branch so that the bug is fixed. I'll try to rewrite patch 3 along the lines Dan suggested. Kevin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PULL 5/5] ui: remove support for SDL1.2 in favour of SDL2
From: Daniel P. Berrangé
SDL1.2 was deprecated in the 2.12.0 release with:
commit e52c6ba34149b4f39c3fd60e59ee32b809db2bfa
Author: Daniel P. Berrange
Date: Mon Jan 15 14:25:33 2018 +
ui: deprecate use of SDL 1.2 in favour of 2.0 series
The SDL 2.0 release was made in Aug, 2013:
https://www.libsdl.org/release/
That will soon be 4 + 1/2 years ago, which is enough time to consider
the 2.0 series widely supported.
Thus we deprecate the SDL 1.2 support, which will allow us to delete it
in the last release of 2018. By this time, SDL 2.0 will be more than 5
years old.
Signed-off-by: Daniel P. Berrange
Reviewed-by: Marc-André Lureau
Message-id: 20180115142533.24585-1-berra...@redhat.com
Signed-off-by: Gerd Hoffmann
It is thus able to be removed in the 3.1.0 release.
Signed-off-by: Daniel P. Berrangé
Message-Id: <20180822131554.3398-4-berra...@redhat.com>
[ kraxel: rebase ]
Signed-off-by: Gerd Hoffmann
---
configure | 60 +--
ui/sdl_zoom.h | 25 --
ui/sdl_zoom_template.h | 219 ---
ui/sdl.c | 1027
ui/sdl_zoom.c | 93 -
qemu-deprecated.texi |9 -
ui/Makefile.objs |5 -
7 files changed, 7 insertions(+), 1431 deletions(-)
delete mode 100644 ui/sdl_zoom.h
delete mode 100644 ui/sdl_zoom_template.h
delete mode 100644 ui/sdl.c
delete mode 100644 ui/sdl_zoom.c
diff --git a/configure b/configure
index c7024d6662..b229f43334 100755
--- a/configure
+++ b/configure
@@ -348,7 +348,6 @@ docs=""
fdt=""
netmap="no"
sdl=""
-sdlabi=""
sdl_image=""
virtfs=""
mpath=""
@@ -577,7 +576,6 @@ query_pkg_config() {
"${pkg_config_exe}" ${QEMU_PKG_CONFIG_FLAGS} "$@"
}
pkg_config=query_pkg_config
-sdl_config="${SDL_CONFIG-${cross_prefix}sdl-config}"
sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}"
# If the user hasn't specified ARFLAGS, default to 'rv', just as make does.
@@ -1044,8 +1042,6 @@ for opt do
;;
--enable-sdl) sdl="yes"
;;
- --with-sdlabi=*) sdlabi="$optarg"
- ;;
--disable-sdl-image) sdl_image="no"
;;
--enable-sdl-image) sdl_image="yes"
@@ -1711,7 +1707,6 @@ disabled with --disable-FEATURE, default is enabled if
available:
nettle nettle cryptography support
gcrypt libgcrypt cryptography support
sdl SDL UI
- --with-sdlabi select preferred SDL ABI 1.2 or 2.0
sdl_image SDL Image support for icons
gtk gtk UI
vte vte support for the gtk UI
@@ -2927,37 +2922,11 @@ fi
sdl_probe ()
{
- sdl_too_old=no
- if test "$sdlabi" = ""; then
- if $pkg_config --exists "sdl2"; then
- sdlabi=2.0
- elif $pkg_config --exists "sdl"; then
- sdlabi=1.2
- else
- sdlabi=2.0
- fi
- fi
-
- if test $sdlabi = "2.0"; then
- sdl_config=$sdl2_config
- sdlname=sdl2
- sdlconfigname=sdl2_config
- elif test $sdlabi = "1.2"; then
- sdlname=sdl
- sdlconfigname=sdl_config
- else
- error_exit "Unknown sdlabi $sdlabi, must be 1.2 or 2.0"
- fi
-
- if test "$(basename $sdl_config)" != $sdlconfigname && ! has ${sdl_config};
then
-sdl_config=$sdlconfigname
- fi
-
- if $pkg_config $sdlname --exists; then
-sdlconfig="$pkg_config $sdlname"
+ if $pkg_config sdl2 --exists; then
+sdlconfig="$pkg_config sdl2"
sdlversion=$($sdlconfig --modversion 2>/dev/null)
elif has ${sdl_config}; then
-sdlconfig="$sdl_config"
+sdlconfig="$sdl2_config"
sdlversion=$($sdlconfig --version)
else
if test "$sdl" = "yes" ; then
@@ -2979,8 +2948,8 @@ EOF
sdl_cflags=$($sdlconfig --cflags 2>/dev/null)
sdl_cflags="$sdl_cflags -Wno-undef" # workaround 2.0.8 bug
if test "$static" = "yes" ; then
-if $pkg_config $sdlname --exists; then
- sdl_libs=$($pkg_config $sdlname --static --libs 2>/dev/null)
+if $pkg_config sdl2 --exists; then
+ sdl_libs=$($pkg_config sdl2 --static --libs 2>/dev/null)
else
sdl_libs=$($sdlconfig --static-libs 2>/dev/null)
fi
@@ -2988,11 +2957,7 @@ EOF
sdl_libs=$($sdlconfig --libs 2>/dev/null)
fi
if compile_prog "$sdl_cflags" "$sdl_libs" ; then
-if test $(echo $sdlversion | sed 's/[^0-9]//g') -lt 121 ; then
- sdl_too_old=yes
-else
- sdl=yes
-fi
+sdl=yes
# static link with sdl ? (note: sdl.pc's --static --libs is broken)
if test "$sdl" = "yes" -a "$static" = "yes" ; then
@@ -3008,7 +2973,7 @@ EOF
fi # static link
else # sdl not found
if test "$sdl" = "yes" ; then
- feature_not_found "sdl" "Install SDL devel"
+ feature_not_found "sdl" "Install SDL2 devel"
fi
sdl=no
fi # sdl compile test
@@ -6220,16 +6185,6 @@ echo "docker$docker"
echo "libpmem support $libpmem"
echo "libudev $libudev"
-if test "$sdl_too_old" = "yes"; then
-echo "-> Your SDL version is too old - please
[libvirt] [PULL 2/5] configure: LM32 Milkymist Texture Mapping Unit (tmu2) also depends of X11
From: Philippe Mathieu-Daudé Commit 5f9b1e35060b8 remove the dependency between OpenGL and X11. However the milkymist-tmu2 device do require X11. When using SDL, the configure script sets need_x11=yes, so the X11 flags are populated to the makefiles. When building without SDL, X11 is not pulled and populated, leading to a link failure: LINKlm32-softmmu/qemu-system-lm32 hw/lm32/milkymist.o: In function `milkymist_tmu2_create': hw/lm32/milkymist-hw.h:114: undefined reference to `XOpenDisplay' hw/lm32/milkymist-hw.h:140: undefined reference to `XFree' hw/lm32/milkymist-hw.h:141: undefined reference to `XCloseDisplay' hw/lm32/milkymist-hw.h:130: undefined reference to `XCloseDisplay' ../hw/display/milkymist-tmu2.o: In function `tmu2_glx_init': hw/display/milkymist-tmu2.c:112: undefined reference to `XOpenDisplay' hw/display/milkymist-tmu2.c:123: undefined reference to `XFree' collect2: error: ld returned 1 exit status gmake[1]: *** [Makefile:199: qemu-system-lm32] Error 1 Enforce the X11 dependency when the LM32 target is built. This will allow us to build QEMU without SDL. Signed-off-by: Philippe Mathieu-Daudé Message-id: 20190130120005.23123-3-phi...@redhat.com Signed-off-by: Gerd Hoffmann --- configure | 10 ++ 1 file changed, 10 insertions(+) diff --git a/configure b/configure index b18281c61f..c7024d6662 100755 --- a/configure +++ b/configure @@ -4047,6 +4047,16 @@ EOF fi fi +if test "$opengl" = "yes" -a "$have_x11" = "yes"; then + for target in $target_list; do +case $target in + lm32-softmmu) # milkymist-tmu2 requires X11 and OpenGL +need_x11=yes + ;; +esac + done +fi + ## # libxml2 probe if test "$libxml2" != "no" ; then -- 2.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PULL 4/5] hw/display/milkymist-tmu2: Move inlined code from header to source
From: Philippe Mathieu-Daudé
Move the complexity of milkymist_tmu2_create() into the
source file. Doing so we avoid to include the X11/OpenGL
headers in all LM32 devices, and we also avoid the duplicate
declaration of glx_fbconfig_attr[] (it is already declared
in hw/display/milkymist-tmu2.c).
Since TYPE_MILKYMIST_TMU2 is now accessible, use it.
Signed-off-by: Philippe Mathieu-Daudé
Message-id: 20190130120005.23123-5-phi...@redhat.com
Signed-off-by: Gerd Hoffmann
---
hw/lm32/milkymist-hw.h | 63 -
include/hw/display/milkymist_tmu2.h | 41
hw/display/milkymist-tmu2.c | 49 +
hw/lm32/milkymist.c | 1 +
MAINTAINERS | 1 +
5 files changed, 92 insertions(+), 63 deletions(-)
create mode 100644 include/hw/display/milkymist_tmu2.h
diff --git a/hw/lm32/milkymist-hw.h b/hw/lm32/milkymist-hw.h
index 32c344ef9f..976cf9254d 100644
--- a/hw/lm32/milkymist-hw.h
+++ b/hw/lm32/milkymist-hw.h
@@ -88,69 +88,6 @@ static inline DeviceState *milkymist_pfpu_create(hwaddr base,
return dev;
}
-#if defined(CONFIG_X11) && defined(CONFIG_OPENGL)
-#include
-#include
-#include
-static const int glx_fbconfig_attr[] = {
-GLX_GREEN_SIZE, 5,
-GLX_GREEN_SIZE, 6,
-GLX_BLUE_SIZE, 5,
-None
-};
-#endif
-
-static inline DeviceState *milkymist_tmu2_create(hwaddr base,
-qemu_irq irq)
-{
-#if defined(CONFIG_X11) && defined(CONFIG_OPENGL)
-DeviceState *dev;
-Display *d;
-GLXFBConfig *configs;
-int nelements;
-int ver_major, ver_minor;
-
-/* check that GLX will work */
-d = XOpenDisplay(NULL);
-if (d == NULL) {
-return NULL;
-}
-
-if (!glXQueryVersion(d, _major, _minor)) {
-/* Yeah, sometimes getting the GLX version can fail.
- * Isn't X beautiful? */
-XCloseDisplay(d);
-return NULL;
-}
-
-if ((ver_major < 1) || ((ver_major == 1) && (ver_minor < 3))) {
-printf("Your GLX version is %d.%d,"
- "but TMU emulation needs at least 1.3. TMU disabled.\n",
- ver_major, ver_minor);
-XCloseDisplay(d);
-return NULL;
-}
-
-configs = glXChooseFBConfig(d, 0, glx_fbconfig_attr, );
-if (configs == NULL) {
-XCloseDisplay(d);
-return NULL;
-}
-
-XFree(configs);
-XCloseDisplay(d);
-
-dev = qdev_create(NULL, "milkymist-tmu2");
-qdev_init_nofail(dev);
-sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
-sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq);
-
-return dev;
-#else
-return NULL;
-#endif
-}
-
static inline DeviceState *milkymist_ac97_create(hwaddr base,
qemu_irq crrequest_irq, qemu_irq crreply_irq, qemu_irq dmar_irq,
qemu_irq dmaw_irq)
diff --git a/include/hw/display/milkymist_tmu2.h
b/include/hw/display/milkymist_tmu2.h
new file mode 100644
index 00..148a119a1d
--- /dev/null
+++ b/include/hw/display/milkymist_tmu2.h
@@ -0,0 +1,41 @@
+/*
+ * QEMU model of the Milkymist texture mapping unit.
+ *
+ * Copyright (c) 2010 Michael Walle
+ * Copyright (c) 2010 Sebastien Bourdeauducq
+ *
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see
[libvirt] [PULL 1/5] hw/display: Move Milkymist specific hardware out of common-obj list
From: Philippe Mathieu-Daudé The Milkymist specific hardware is only used by the LM32 target, it is pointless to compile those objects in other targets. Signed-off-by: Philippe Mathieu-Daudé Message-id: 20190130120005.23123-2-phi...@redhat.com Signed-off-by: Gerd Hoffmann --- hw/display/Makefile.objs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs index 97acd5b6cb..5b770817c7 100644 --- a/hw/display/Makefile.objs +++ b/hw/display/Makefile.objs @@ -25,10 +25,10 @@ common-obj-$(CONFIG_BOCHS_DISPLAY) += edid-region.o common-obj-$(CONFIG_BLIZZARD) += blizzard.o common-obj-$(CONFIG_EXYNOS4) += exynos4210_fimd.o common-obj-$(CONFIG_FRAMEBUFFER) += framebuffer.o -common-obj-$(CONFIG_MILKYMIST) += milkymist-vgafb.o +obj-$(CONFIG_MILKYMIST) += milkymist-vgafb.o common-obj-$(CONFIG_ZAURUS) += tc6393xb.o -common-obj-$(CONFIG_MILKYMIST_TMU2) += milkymist-tmu2.o +obj-$(CONFIG_MILKYMIST_TMU2) += milkymist-tmu2.o milkymist-tmu2.o-cflags := $(X11_CFLAGS) milkymist-tmu2.o-libs := $(X11_LIBS) -- 2.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PULL 3/5] hw/display/milkymist-tmu2: Explicit the dependency to both X11 / OpenGL
From: Philippe Mathieu-Daudé
The TMU device requires both X11 and OpenGL.
Signed-off-by: Philippe Mathieu-Daudé
Message-id: 20190130120005.23123-4-phi...@redhat.com
Signed-off-by: Gerd Hoffmann
---
hw/lm32/milkymist-hw.h | 4 ++--
default-configs/lm32-softmmu.mak | 2 +-
hw/display/Makefile.objs | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/lm32/milkymist-hw.h b/hw/lm32/milkymist-hw.h
index d3be0cfb3a..32c344ef9f 100644
--- a/hw/lm32/milkymist-hw.h
+++ b/hw/lm32/milkymist-hw.h
@@ -88,7 +88,7 @@ static inline DeviceState *milkymist_pfpu_create(hwaddr base,
return dev;
}
-#ifdef CONFIG_OPENGL
+#if defined(CONFIG_X11) && defined(CONFIG_OPENGL)
#include
#include
#include
@@ -103,7 +103,7 @@ static const int glx_fbconfig_attr[] = {
static inline DeviceState *milkymist_tmu2_create(hwaddr base,
qemu_irq irq)
{
-#ifdef CONFIG_OPENGL
+#if defined(CONFIG_X11) && defined(CONFIG_OPENGL)
DeviceState *dev;
Display *d;
GLXFBConfig *configs;
diff --git a/default-configs/lm32-softmmu.mak b/default-configs/lm32-softmmu.mak
index 4889348a10..4049b23562 100644
--- a/default-configs/lm32-softmmu.mak
+++ b/default-configs/lm32-softmmu.mak
@@ -2,7 +2,7 @@
CONFIG_LM32=y
CONFIG_MILKYMIST=y
-CONFIG_MILKYMIST_TMU2=$(CONFIG_OPENGL)
+CONFIG_MILKYMIST_TMU2=$(call land,$(CONFIG_X11),$(CONFIG_OPENGL))
CONFIG_FRAMEBUFFER=y
CONFIG_PTIMER=y
CONFIG_PFLASH_CFI01=y
diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
index 5b770817c7..7c4ae9a0fd 100644
--- a/hw/display/Makefile.objs
+++ b/hw/display/Makefile.objs
@@ -29,8 +29,8 @@ obj-$(CONFIG_MILKYMIST) += milkymist-vgafb.o
common-obj-$(CONFIG_ZAURUS) += tc6393xb.o
obj-$(CONFIG_MILKYMIST_TMU2) += milkymist-tmu2.o
-milkymist-tmu2.o-cflags := $(X11_CFLAGS)
-milkymist-tmu2.o-libs := $(X11_LIBS)
+milkymist-tmu2.o-cflags := $(X11_CFLAGS) $(OPENGL_CFLAGS)
+milkymist-tmu2.o-libs := $(X11_LIBS) $(OPENGL_LIBS)
obj-$(CONFIG_OMAP) += omap_dss.o
obj-$(CONFIG_OMAP) += omap_lcdc.o
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PULL 0/5] Ui 20190201 patches
The following changes since commit e8977901b79fb678f288dd372261b640bbeccd0d: Merge remote-tracking branch 'remotes/vivier2/tags/trivial-branch-pull-request' into staging (2019-01-31 15:40:39 +) are available in the git repository at: git://git.kraxel.org/qemu tags/ui-20190201-pull-request for you to fetch changes up to 0015ca5cbabe0b31d31610ddfaafd90a9e5911a4: ui: remove support for SDL1.2 in favour of SDL2 (2019-02-01 11:59:12 +0100) ui: fix build with SDL disabled, drop SDL1 support. Daniel P. Berrangé (1): ui: remove support for SDL1.2 in favour of SDL2 Philippe Mathieu-Daudé (4): hw/display: Move Milkymist specific hardware out of common-obj list configure: LM32 Milkymist Texture Mapping Unit (tmu2) also depends of X11 hw/display/milkymist-tmu2: Explicit the dependency to both X11 / OpenGL hw/display/milkymist-tmu2: Move inlined code from header to source configure | 70 +-- hw/lm32/milkymist-hw.h | 63 --- include/hw/display/milkymist_tmu2.h | 41 ++ ui/sdl_zoom.h | 25 - ui/sdl_zoom_template.h | 219 hw/display/milkymist-tmu2.c | 49 ++ hw/lm32/milkymist.c |1 + ui/sdl.c| 1027 --- ui/sdl_zoom.c | 93 MAINTAINERS |1 + default-configs/lm32-softmmu.mak|2 +- hw/display/Makefile.objs|8 +- qemu-deprecated.texi|9 - ui/Makefile.objs|5 - 14 files changed, 114 insertions(+), 1499 deletions(-) create mode 100644 include/hw/display/milkymist_tmu2.h delete mode 100644 ui/sdl_zoom.h delete mode 100644 ui/sdl_zoom_template.h delete mode 100644 ui/sdl.c delete mode 100644 ui/sdl_zoom.c -- 2.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: Update qemucaps2xml for QEMU 4.0.0 on x86_64
On Fri, 2019-02-01 at 12:14 +0100, Ján Tomko wrote: > On Thu, Jan 31, 2019 at 02:22:48PM +0100, Andrea Bolognani wrote: > > Commit fb0d0d6c5492 added capabilities data and updated > > qemucapabilitiestest but forgot to update qemucaps2xmltest > > at the same time. > > > > Signed-off-by: Andrea Bolognani > > --- > > *cough* and of course the reviewer didn't notice *cough* > > It's okay, it does not seem THAT useful: > > $ sha1sum tests/qemucaps2xmloutdata/caps_* | sort > 0920190f4cc69b09208d040bfea7e8c28aa5d399 > tests/qemucaps2xmloutdata/caps_2.10.0.ppc64.xml > 0920190f4cc69b09208d040bfea7e8c28aa5d399 > tests/qemucaps2xmloutdata/caps_2.12.0.ppc64.xml > 0920190f4cc69b09208d040bfea7e8c28aa5d399 > tests/qemucaps2xmloutdata/caps_2.6.0.ppc64.xml > 0920190f4cc69b09208d040bfea7e8c28aa5d399 > tests/qemucaps2xmloutdata/caps_2.9.0.ppc64.xml > 0920190f4cc69b09208d040bfea7e8c28aa5d399 > tests/qemucaps2xmloutdata/caps_3.0.0.ppc64.xml > 0920190f4cc69b09208d040bfea7e8c28aa5d399 > tests/qemucaps2xmloutdata/caps_3.1.0.ppc64.xml > 30e84ffb64dbbf5e98401cc48eb8d62718d9f909 > tests/qemucaps2xmloutdata/caps_2.10.0.aarch64.xml > 30e84ffb64dbbf5e98401cc48eb8d62718d9f909 > tests/qemucaps2xmloutdata/caps_2.12.0.aarch64.xml > 30e84ffb64dbbf5e98401cc48eb8d62718d9f909 > tests/qemucaps2xmloutdata/caps_2.6.0.aarch64.xml > 7215a9dc383575f64f379125d41ade537ff56ddc > tests/qemucaps2xmloutdata/caps_1.5.3.x86_64.xml > 7215a9dc383575f64f379125d41ade537ff56ddc > tests/qemucaps2xmloutdata/caps_1.6.0.x86_64.xml > 7215a9dc383575f64f379125d41ade537ff56ddc > tests/qemucaps2xmloutdata/caps_1.7.0.x86_64.xml > 7215a9dc383575f64f379125d41ade537ff56ddc > tests/qemucaps2xmloutdata/caps_2.10.0.x86_64.xml > 7215a9dc383575f64f379125d41ade537ff56ddc > tests/qemucaps2xmloutdata/caps_2.11.0.x86_64.xml > 7215a9dc383575f64f379125d41ade537ff56ddc > tests/qemucaps2xmloutdata/caps_2.1.1.x86_64.xml > 7215a9dc383575f64f379125d41ade537ff56ddc > tests/qemucaps2xmloutdata/caps_2.12.0.x86_64.xml > 7215a9dc383575f64f379125d41ade537ff56ddc > tests/qemucaps2xmloutdata/caps_2.4.0.x86_64.xml > 7215a9dc383575f64f379125d41ade537ff56ddc > tests/qemucaps2xmloutdata/caps_2.5.0.x86_64.xml > 7215a9dc383575f64f379125d41ade537ff56ddc > tests/qemucaps2xmloutdata/caps_2.6.0.x86_64.xml > 7215a9dc383575f64f379125d41ade537ff56ddc > tests/qemucaps2xmloutdata/caps_2.7.0.x86_64.xml > 7215a9dc383575f64f379125d41ade537ff56ddc > tests/qemucaps2xmloutdata/caps_2.8.0.x86_64.xml > 7215a9dc383575f64f379125d41ade537ff56ddc > tests/qemucaps2xmloutdata/caps_2.9.0.x86_64.xml > 7215a9dc383575f64f379125d41ade537ff56ddc > tests/qemucaps2xmloutdata/caps_3.0.0.x86_64.xml > 7215a9dc383575f64f379125d41ade537ff56ddc > tests/qemucaps2xmloutdata/caps_3.1.0.x86_64.xml > 7215a9dc383575f64f379125d41ade537ff56ddc > tests/qemucaps2xmloutdata/caps_4.0.0.x86_64.xml > 748474f70fe5743630548e2317aa57ba61e3436c > tests/qemucaps2xmloutdata/caps_3.0.0.riscv64.xml > 87e692398a85c24540ae3d4d42ba3f55bb7b6769 > tests/qemucaps2xmloutdata/caps_2.10.0.s390x.xml > 87e692398a85c24540ae3d4d42ba3f55bb7b6769 > tests/qemucaps2xmloutdata/caps_2.11.0.s390x.xml > 87e692398a85c24540ae3d4d42ba3f55bb7b6769 > tests/qemucaps2xmloutdata/caps_2.12.0.s390x.xml > 87e692398a85c24540ae3d4d42ba3f55bb7b6769 > tests/qemucaps2xmloutdata/caps_2.7.0.s390x.xml > 87e692398a85c24540ae3d4d42ba3f55bb7b6769 > tests/qemucaps2xmloutdata/caps_2.8.0.s390x.xml > 87e692398a85c24540ae3d4d42ba3f55bb7b6769 > tests/qemucaps2xmloutdata/caps_2.9.0.s390x.xml > 87e692398a85c24540ae3d4d42ba3f55bb7b6769 > tests/qemucaps2xmloutdata/caps_3.0.0.s390x.xml > 9d061fd737dca36839ee39fdabe23b7e223b5afd > tests/qemucaps2xmloutdata/caps_3.0.0.riscv32.xml Solid point! Addressed in https://www.redhat.com/archives/libvir-list/2019-February/msg00026.html -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] tests: Unify qemucaps2xml output files
Turns out different versions of QEMU on the same architecture
produce the same output, so we can have a single output file
per architecture instead of duplicating the same data over and
over again.
Spotted-by: Ján Tomko
Signed-off-by: Andrea Bolognani
---
...ps_2.10.0.aarch64.xml => caps.aarch64.xml} | 0
.../{caps_2.10.0.ppc64.xml => caps.ppc64.xml} | 0
...aps_3.0.0.riscv32.xml => caps.riscv32.xml} | 0
...aps_3.0.0.riscv64.xml => caps.riscv64.xml} | 0
.../{caps_2.10.0.s390x.xml => caps.s390x.xml} | 0
...{caps_1.5.3.x86_64.xml => caps.x86_64.xml} | 0
.../qemucaps2xmloutdata/caps_1.6.0.x86_64.xml | 28 ---
.../qemucaps2xmloutdata/caps_1.7.0.x86_64.xml | 28 ---
.../qemucaps2xmloutdata/caps_2.1.1.x86_64.xml | 28 ---
.../caps_2.10.0.x86_64.xml| 28 ---
.../qemucaps2xmloutdata/caps_2.11.0.s390x.xml | 26 -
.../caps_2.11.0.x86_64.xml| 28 ---
.../caps_2.12.0.aarch64.xml | 27 --
.../qemucaps2xmloutdata/caps_2.12.0.ppc64.xml | 26 -
.../qemucaps2xmloutdata/caps_2.12.0.s390x.xml | 26 -
.../caps_2.12.0.x86_64.xml| 28 ---
.../qemucaps2xmloutdata/caps_2.4.0.x86_64.xml | 28 ---
.../qemucaps2xmloutdata/caps_2.5.0.x86_64.xml | 28 ---
.../caps_2.6.0.aarch64.xml| 27 --
.../qemucaps2xmloutdata/caps_2.6.0.ppc64.xml | 26 -
.../qemucaps2xmloutdata/caps_2.6.0.x86_64.xml | 28 ---
.../qemucaps2xmloutdata/caps_2.7.0.s390x.xml | 26 -
.../qemucaps2xmloutdata/caps_2.7.0.x86_64.xml | 28 ---
.../qemucaps2xmloutdata/caps_2.8.0.s390x.xml | 26 -
.../qemucaps2xmloutdata/caps_2.8.0.x86_64.xml | 28 ---
.../qemucaps2xmloutdata/caps_2.9.0.ppc64.xml | 26 -
.../qemucaps2xmloutdata/caps_2.9.0.s390x.xml | 26 -
.../qemucaps2xmloutdata/caps_2.9.0.x86_64.xml | 28 ---
.../qemucaps2xmloutdata/caps_3.0.0.ppc64.xml | 26 -
.../qemucaps2xmloutdata/caps_3.0.0.s390x.xml | 26 -
.../qemucaps2xmloutdata/caps_3.0.0.x86_64.xml | 28 ---
.../qemucaps2xmloutdata/caps_3.1.0.ppc64.xml | 26 -
.../qemucaps2xmloutdata/caps_3.1.0.x86_64.xml | 28 ---
.../caps_4.0.0.riscv32.xml| 25 -
.../caps_4.0.0.riscv64.xml| 25 -
.../qemucaps2xmloutdata/caps_4.0.0.x86_64.xml | 28 ---
tests/qemucaps2xmltest.c | 4 +--
37 files changed, 2 insertions(+), 812 deletions(-)
rename tests/qemucaps2xmloutdata/{caps_2.10.0.aarch64.xml => caps.aarch64.xml}
(100%)
rename tests/qemucaps2xmloutdata/{caps_2.10.0.ppc64.xml => caps.ppc64.xml}
(100%)
rename tests/qemucaps2xmloutdata/{caps_3.0.0.riscv32.xml => caps.riscv32.xml}
(100%)
rename tests/qemucaps2xmloutdata/{caps_3.0.0.riscv64.xml => caps.riscv64.xml}
(100%)
rename tests/qemucaps2xmloutdata/{caps_2.10.0.s390x.xml => caps.s390x.xml}
(100%)
rename tests/qemucaps2xmloutdata/{caps_1.5.3.x86_64.xml => caps.x86_64.xml}
(100%)
delete mode 100644 tests/qemucaps2xmloutdata/caps_1.6.0.x86_64.xml
delete mode 100644 tests/qemucaps2xmloutdata/caps_1.7.0.x86_64.xml
delete mode 100644 tests/qemucaps2xmloutdata/caps_2.1.1.x86_64.xml
delete mode 100644 tests/qemucaps2xmloutdata/caps_2.10.0.x86_64.xml
delete mode 100644 tests/qemucaps2xmloutdata/caps_2.11.0.s390x.xml
delete mode 100644 tests/qemucaps2xmloutdata/caps_2.11.0.x86_64.xml
delete mode 100644 tests/qemucaps2xmloutdata/caps_2.12.0.aarch64.xml
delete mode 100644 tests/qemucaps2xmloutdata/caps_2.12.0.ppc64.xml
delete mode 100644 tests/qemucaps2xmloutdata/caps_2.12.0.s390x.xml
delete mode 100644 tests/qemucaps2xmloutdata/caps_2.12.0.x86_64.xml
delete mode 100644 tests/qemucaps2xmloutdata/caps_2.4.0.x86_64.xml
delete mode 100644 tests/qemucaps2xmloutdata/caps_2.5.0.x86_64.xml
delete mode 100644 tests/qemucaps2xmloutdata/caps_2.6.0.aarch64.xml
delete mode 100644 tests/qemucaps2xmloutdata/caps_2.6.0.ppc64.xml
delete mode 100644 tests/qemucaps2xmloutdata/caps_2.6.0.x86_64.xml
delete mode 100644 tests/qemucaps2xmloutdata/caps_2.7.0.s390x.xml
delete mode 100644 tests/qemucaps2xmloutdata/caps_2.7.0.x86_64.xml
delete mode 100644 tests/qemucaps2xmloutdata/caps_2.8.0.s390x.xml
delete mode 100644 tests/qemucaps2xmloutdata/caps_2.8.0.x86_64.xml
delete mode 100644 tests/qemucaps2xmloutdata/caps_2.9.0.ppc64.xml
delete mode 100644 tests/qemucaps2xmloutdata/caps_2.9.0.s390x.xml
delete mode 100644 tests/qemucaps2xmloutdata/caps_2.9.0.x86_64.xml
delete mode 100644 tests/qemucaps2xmloutdata/caps_3.0.0.ppc64.xml
delete mode 100644 tests/qemucaps2xmloutdata/caps_3.0.0.s390x.xml
delete mode 100644
[libvirt] [PATCH] docs: news: Update the release notes with the SEV permission fix
Signed-off-by: Erik Skultety --- docs/news.xml | 12 1 file changed, 12 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 55d6a3926b..fcc42698b3 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -50,6 +50,18 @@ + + + qemu: Use CAP_DAC_OVERRIDE during QEMU capabilities probing + + + By default, libvirt runs the QEMU process as qemu:qemu which could + cause issues during probing as some features (like AMD SEV) might be + inaccesible to QEMU because of file system permissions. Therefore, + CAP_DAC_OVERRIDE is granted to overcome these for the purposes of + probing. + + storage: Add default mount options for fs/netfs storage pools -- 2.20.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/5] qemu: caps: Use CAP_DAC_OVERRIDE for probing to avoid permission issues
On Fri, Feb 01, 2019 at 12:33:19PM +0100, Erik Skultety wrote:
> On Fri, Feb 01, 2019 at 10:31:52AM +, Daniel P. Berrangé wrote:
> > On Thu, Jan 31, 2019 at 04:26:18PM +0100, Erik Skultety wrote:
> > > This is mainly about /dev/sev and its default permissions 0600. Of
> > > course, rule of 'tinfoil' would be that we can't trust anything, but the
> > > probing code in QEMU is considered safe from security's perspective + we
> > > can't create an udev rule for this at the moment, because ioctls and
> > > filesystem permisions are cross checked in kernel and therefore a user
> > > with read permisions could issue a 'privileged' operation on SEV which
> > > is currently only limited to root.
> > >
> > > Signed-off-by: Erik Skultety
> > > ---
> > > src/qemu/qemu_capabilities.c | 11 +++
> > > src/util/virutil.c | 31 +--
> > > 2 files changed, 40 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> > > index 5cf4b617c6..2e84c965e8 100644
> > > --- a/src/qemu/qemu_capabilities.c
> > > +++ b/src/qemu/qemu_capabilities.c
> > > @@ -53,6 +53,10 @@
> > > #include
> > > #include
> > >
> > > +#if WITH_CAPNG
> > > +# include
> > > +#endif
> > > +
> > > #define VIR_FROM_THIS VIR_FROM_QEMU
> > >
> > > VIR_LOG_INIT("qemu.qemu_capabilities");
> > > @@ -4515,6 +4519,13 @@
> > > virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd,
> > > NULL);
> > > virCommandAddEnvPassCommon(cmd->cmd);
> > > virCommandClearCaps(cmd->cmd);
> > > +
> > > +#if WITH_CAPNG
> > > +/* QEMU might run into permission issues, e.g. /dev/sev (0600),
> > > override
> > > + * them just for the purpose of probing */
> > > +virCommandAllowCap(cmd->cmd, CAP_DAC_OVERRIDE);
> > > +#endif
> > > +
> > > virCommandSetGID(cmd->cmd, cmd->runGid);
> > > virCommandSetUID(cmd->cmd, cmd->runUid);
> > >
> > > diff --git a/src/util/virutil.c b/src/util/virutil.c
> > > index 5251b66454..02de92061c 100644
> > > --- a/src/util/virutil.c
> > > +++ b/src/util/virutil.c
> > > @@ -1502,8 +1502,10 @@ virSetUIDGIDWithCaps(uid_t uid, gid_t gid, gid_t
> > > *groups, int ngroups,
> > > {
> > > size_t i;
> > > int capng_ret, ret = -1;
> > > -bool need_setgid = false, need_setuid = false;
> > > +bool need_setgid = false;
> > > +bool need_setuid = false;
> > > bool need_setpcap = false;
> > > +const char *capstr = NULL;
> > >
> > > /* First drop all caps (unless the requested uid is "unchanged" or
> > > * root and clearExistingCaps wasn't requested), then add back
> > > @@ -1512,14 +1514,18 @@ virSetUIDGIDWithCaps(uid_t uid, gid_t gid, gid_t
> > > *groups, int ngroups,
> > > */
> > >
> > > if (clearExistingCaps || (uid != (uid_t)-1 && uid != 0))
> > > - capng_clear(CAPNG_SELECT_BOTH);
> > > +capng_clear(CAPNG_SELECT_BOTH);
> > >
> > > for (i = 0; i <= CAP_LAST_CAP; i++) {
> > > +capstr = capng_capability_to_name(i);
> > > +
> > > if (capBits & (1ULL << i)) {
> > > capng_update(CAPNG_ADD,
> > > CAPNG_EFFECTIVE|CAPNG_INHERITABLE|
> > > CAPNG_PERMITTED|CAPNG_BOUNDING_SET,
> > > i);
> > > +
> > > +VIR_DEBUG("Added '%s' to child capabilities' set", capstr);
> > > }
> > > }
> > >
> > > @@ -1579,6 +1585,27 @@ virSetUIDGIDWithCaps(uid_t uid, gid_t gid, gid_t
> > > *groups, int ngroups,
> > > goto cleanup;
> > > }
> > >
> > > +# ifdef PR_CAP_AMBIENT
> > > +/* we couldn't do this in the loop earlier above, because the
> > > capabilities
> > > + * were not applied yet, since in order to add a capability into the
> > > AMBIENT
> > > + * set, it has to be present in both the PERMITTED and INHERITABLE
> > > sets
> > > + * (capabilities(7))
> > > + */
> > > +for (i = 0; i <= CAP_LAST_CAP; i++) {
> > > +capstr = capng_capability_to_name(i);
> > > +
> > > +if (capBits & (1ULL << i)) {
> > > +if (prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_RAISE, i, 0, 0) <
> > > 0) {
> > > +virReportSystemError(errno,
> > > + _("prctl failed to enable '%s' in
> > > the "
> > > + "AMBIENT set"),
> > > + capstr);
> > > +goto cleanup;
> > > +}
> > > +}
> > > +}
> > > +# endif
> >
> > This is set a bit earlier than I set it in my PoC patch, but I'll assume
> > it still works given the comment you added.
>
> I was trying to understand whether there was a particular reason why you added
> it to the ambient set later, so my first lame attempt was to merge the 2 'for'
> loops into 1, since they were identical apart from the prctl syscall which led
> to an error. So I investigated and found the
Re: [libvirt] [PATCH 5/5] qemu: caps: Use CAP_DAC_OVERRIDE for probing to avoid permission issues
On Fri, Feb 01, 2019 at 10:31:52AM +, Daniel P. Berrangé wrote:
> On Thu, Jan 31, 2019 at 04:26:18PM +0100, Erik Skultety wrote:
> > This is mainly about /dev/sev and its default permissions 0600. Of
> > course, rule of 'tinfoil' would be that we can't trust anything, but the
> > probing code in QEMU is considered safe from security's perspective + we
> > can't create an udev rule for this at the moment, because ioctls and
> > filesystem permisions are cross checked in kernel and therefore a user
> > with read permisions could issue a 'privileged' operation on SEV which
> > is currently only limited to root.
> >
> > Signed-off-by: Erik Skultety
> > ---
> > src/qemu/qemu_capabilities.c | 11 +++
> > src/util/virutil.c | 31 +--
> > 2 files changed, 40 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> > index 5cf4b617c6..2e84c965e8 100644
> > --- a/src/qemu/qemu_capabilities.c
> > +++ b/src/qemu/qemu_capabilities.c
> > @@ -53,6 +53,10 @@
> > #include
> > #include
> >
> > +#if WITH_CAPNG
> > +# include
> > +#endif
> > +
> > #define VIR_FROM_THIS VIR_FROM_QEMU
> >
> > VIR_LOG_INIT("qemu.qemu_capabilities");
> > @@ -4515,6 +4519,13 @@
> > virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd,
> > NULL);
> > virCommandAddEnvPassCommon(cmd->cmd);
> > virCommandClearCaps(cmd->cmd);
> > +
> > +#if WITH_CAPNG
> > +/* QEMU might run into permission issues, e.g. /dev/sev (0600),
> > override
> > + * them just for the purpose of probing */
> > +virCommandAllowCap(cmd->cmd, CAP_DAC_OVERRIDE);
> > +#endif
> > +
> > virCommandSetGID(cmd->cmd, cmd->runGid);
> > virCommandSetUID(cmd->cmd, cmd->runUid);
> >
> > diff --git a/src/util/virutil.c b/src/util/virutil.c
> > index 5251b66454..02de92061c 100644
> > --- a/src/util/virutil.c
> > +++ b/src/util/virutil.c
> > @@ -1502,8 +1502,10 @@ virSetUIDGIDWithCaps(uid_t uid, gid_t gid, gid_t
> > *groups, int ngroups,
> > {
> > size_t i;
> > int capng_ret, ret = -1;
> > -bool need_setgid = false, need_setuid = false;
> > +bool need_setgid = false;
> > +bool need_setuid = false;
> > bool need_setpcap = false;
> > +const char *capstr = NULL;
> >
> > /* First drop all caps (unless the requested uid is "unchanged" or
> > * root and clearExistingCaps wasn't requested), then add back
> > @@ -1512,14 +1514,18 @@ virSetUIDGIDWithCaps(uid_t uid, gid_t gid, gid_t
> > *groups, int ngroups,
> > */
> >
> > if (clearExistingCaps || (uid != (uid_t)-1 && uid != 0))
> > - capng_clear(CAPNG_SELECT_BOTH);
> > +capng_clear(CAPNG_SELECT_BOTH);
> >
> > for (i = 0; i <= CAP_LAST_CAP; i++) {
> > +capstr = capng_capability_to_name(i);
> > +
> > if (capBits & (1ULL << i)) {
> > capng_update(CAPNG_ADD,
> > CAPNG_EFFECTIVE|CAPNG_INHERITABLE|
> > CAPNG_PERMITTED|CAPNG_BOUNDING_SET,
> > i);
> > +
> > +VIR_DEBUG("Added '%s' to child capabilities' set", capstr);
> > }
> > }
> >
> > @@ -1579,6 +1585,27 @@ virSetUIDGIDWithCaps(uid_t uid, gid_t gid, gid_t
> > *groups, int ngroups,
> > goto cleanup;
> > }
> >
> > +# ifdef PR_CAP_AMBIENT
> > +/* we couldn't do this in the loop earlier above, because the
> > capabilities
> > + * were not applied yet, since in order to add a capability into the
> > AMBIENT
> > + * set, it has to be present in both the PERMITTED and INHERITABLE sets
> > + * (capabilities(7))
> > + */
> > +for (i = 0; i <= CAP_LAST_CAP; i++) {
> > +capstr = capng_capability_to_name(i);
> > +
> > +if (capBits & (1ULL << i)) {
> > +if (prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_RAISE, i, 0, 0) < 0) {
> > +virReportSystemError(errno,
> > + _("prctl failed to enable '%s' in the
> > "
> > + "AMBIENT set"),
> > + capstr);
> > +goto cleanup;
> > +}
> > +}
> > +}
> > +# endif
>
> This is set a bit earlier than I set it in my PoC patch, but I'll assume
> it still works given the comment you added.
I was trying to understand whether there was a particular reason why you added
it to the ambient set later, so my first lame attempt was to merge the 2 'for'
loops into 1, since they were identical apart from the prctl syscall which led
to an error. So I investigated and found the restriction I mentioned in the
comment so I moved it after the caps were first applied and it did work:
(trial-error)+-research method™
>
> Reviewed-by: Daniel P. Berrangé
Thanks,
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: Update qemucaps2xml for QEMU 4.0.0 on x86_64
On Thu, Jan 31, 2019 at 02:22:48PM +0100, Andrea Bolognani wrote: Commit fb0d0d6c5492 added capabilities data and updated qemucapabilitiestest but forgot to update qemucaps2xmltest at the same time. Signed-off-by: Andrea Bolognani --- *cough* and of course the reviewer didn't notice *cough* It's okay, it does not seem THAT useful: $ sha1sum tests/qemucaps2xmloutdata/caps_* | sort 0920190f4cc69b09208d040bfea7e8c28aa5d399 tests/qemucaps2xmloutdata/caps_2.10.0.ppc64.xml 0920190f4cc69b09208d040bfea7e8c28aa5d399 tests/qemucaps2xmloutdata/caps_2.12.0.ppc64.xml 0920190f4cc69b09208d040bfea7e8c28aa5d399 tests/qemucaps2xmloutdata/caps_2.6.0.ppc64.xml 0920190f4cc69b09208d040bfea7e8c28aa5d399 tests/qemucaps2xmloutdata/caps_2.9.0.ppc64.xml 0920190f4cc69b09208d040bfea7e8c28aa5d399 tests/qemucaps2xmloutdata/caps_3.0.0.ppc64.xml 0920190f4cc69b09208d040bfea7e8c28aa5d399 tests/qemucaps2xmloutdata/caps_3.1.0.ppc64.xml 30e84ffb64dbbf5e98401cc48eb8d62718d9f909 tests/qemucaps2xmloutdata/caps_2.10.0.aarch64.xml 30e84ffb64dbbf5e98401cc48eb8d62718d9f909 tests/qemucaps2xmloutdata/caps_2.12.0.aarch64.xml 30e84ffb64dbbf5e98401cc48eb8d62718d9f909 tests/qemucaps2xmloutdata/caps_2.6.0.aarch64.xml 7215a9dc383575f64f379125d41ade537ff56ddc tests/qemucaps2xmloutdata/caps_1.5.3.x86_64.xml 7215a9dc383575f64f379125d41ade537ff56ddc tests/qemucaps2xmloutdata/caps_1.6.0.x86_64.xml 7215a9dc383575f64f379125d41ade537ff56ddc tests/qemucaps2xmloutdata/caps_1.7.0.x86_64.xml 7215a9dc383575f64f379125d41ade537ff56ddc tests/qemucaps2xmloutdata/caps_2.10.0.x86_64.xml 7215a9dc383575f64f379125d41ade537ff56ddc tests/qemucaps2xmloutdata/caps_2.11.0.x86_64.xml 7215a9dc383575f64f379125d41ade537ff56ddc tests/qemucaps2xmloutdata/caps_2.1.1.x86_64.xml 7215a9dc383575f64f379125d41ade537ff56ddc tests/qemucaps2xmloutdata/caps_2.12.0.x86_64.xml 7215a9dc383575f64f379125d41ade537ff56ddc tests/qemucaps2xmloutdata/caps_2.4.0.x86_64.xml 7215a9dc383575f64f379125d41ade537ff56ddc tests/qemucaps2xmloutdata/caps_2.5.0.x86_64.xml 7215a9dc383575f64f379125d41ade537ff56ddc tests/qemucaps2xmloutdata/caps_2.6.0.x86_64.xml 7215a9dc383575f64f379125d41ade537ff56ddc tests/qemucaps2xmloutdata/caps_2.7.0.x86_64.xml 7215a9dc383575f64f379125d41ade537ff56ddc tests/qemucaps2xmloutdata/caps_2.8.0.x86_64.xml 7215a9dc383575f64f379125d41ade537ff56ddc tests/qemucaps2xmloutdata/caps_2.9.0.x86_64.xml 7215a9dc383575f64f379125d41ade537ff56ddc tests/qemucaps2xmloutdata/caps_3.0.0.x86_64.xml 7215a9dc383575f64f379125d41ade537ff56ddc tests/qemucaps2xmloutdata/caps_3.1.0.x86_64.xml 7215a9dc383575f64f379125d41ade537ff56ddc tests/qemucaps2xmloutdata/caps_4.0.0.x86_64.xml 748474f70fe5743630548e2317aa57ba61e3436c tests/qemucaps2xmloutdata/caps_3.0.0.riscv64.xml 87e692398a85c24540ae3d4d42ba3f55bb7b6769 tests/qemucaps2xmloutdata/caps_2.10.0.s390x.xml 87e692398a85c24540ae3d4d42ba3f55bb7b6769 tests/qemucaps2xmloutdata/caps_2.11.0.s390x.xml 87e692398a85c24540ae3d4d42ba3f55bb7b6769 tests/qemucaps2xmloutdata/caps_2.12.0.s390x.xml 87e692398a85c24540ae3d4d42ba3f55bb7b6769 tests/qemucaps2xmloutdata/caps_2.7.0.s390x.xml 87e692398a85c24540ae3d4d42ba3f55bb7b6769 tests/qemucaps2xmloutdata/caps_2.8.0.s390x.xml 87e692398a85c24540ae3d4d42ba3f55bb7b6769 tests/qemucaps2xmloutdata/caps_2.9.0.s390x.xml 87e692398a85c24540ae3d4d42ba3f55bb7b6769 tests/qemucaps2xmloutdata/caps_3.0.0.s390x.xml 9d061fd737dca36839ee39fdabe23b7e223b5afd tests/qemucaps2xmloutdata/caps_3.0.0.riscv32.xml .../qemucaps2xmloutdata/caps_4.0.0.x86_64.xml | 28 +++ tests/qemucaps2xmltest.c | 1 + 2 files changed, 29 insertions(+) create mode 100644 tests/qemucaps2xmloutdata/caps_4.0.0.x86_64.xml Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/5] qemu: caps: Use CAP_DAC_OVERRIDE for probing to avoid permission issues
On Thu, Jan 31, 2019 at 04:26:18PM +0100, Erik Skultety wrote:
> This is mainly about /dev/sev and its default permissions 0600. Of
> course, rule of 'tinfoil' would be that we can't trust anything, but the
> probing code in QEMU is considered safe from security's perspective + we
> can't create an udev rule for this at the moment, because ioctls and
> filesystem permisions are cross checked in kernel and therefore a user
> with read permisions could issue a 'privileged' operation on SEV which
> is currently only limited to root.
>
> Signed-off-by: Erik Skultety
> ---
> src/qemu/qemu_capabilities.c | 11 +++
> src/util/virutil.c | 31 +--
> 2 files changed, 40 insertions(+), 2 deletions(-)
>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 5cf4b617c6..2e84c965e8 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -53,6 +53,10 @@
> #include
> #include
>
> +#if WITH_CAPNG
> +# include
> +#endif
> +
> #define VIR_FROM_THIS VIR_FROM_QEMU
>
> VIR_LOG_INIT("qemu.qemu_capabilities");
> @@ -4515,6 +4519,13 @@
> virQEMUCapsInitQMPCommandRun(virQEMUCapsInitQMPCommandPtr cmd,
> NULL);
> virCommandAddEnvPassCommon(cmd->cmd);
> virCommandClearCaps(cmd->cmd);
> +
> +#if WITH_CAPNG
> +/* QEMU might run into permission issues, e.g. /dev/sev (0600), override
> + * them just for the purpose of probing */
> +virCommandAllowCap(cmd->cmd, CAP_DAC_OVERRIDE);
> +#endif
> +
> virCommandSetGID(cmd->cmd, cmd->runGid);
> virCommandSetUID(cmd->cmd, cmd->runUid);
>
> diff --git a/src/util/virutil.c b/src/util/virutil.c
> index 5251b66454..02de92061c 100644
> --- a/src/util/virutil.c
> +++ b/src/util/virutil.c
> @@ -1502,8 +1502,10 @@ virSetUIDGIDWithCaps(uid_t uid, gid_t gid, gid_t
> *groups, int ngroups,
> {
> size_t i;
> int capng_ret, ret = -1;
> -bool need_setgid = false, need_setuid = false;
> +bool need_setgid = false;
> +bool need_setuid = false;
> bool need_setpcap = false;
> +const char *capstr = NULL;
>
> /* First drop all caps (unless the requested uid is "unchanged" or
> * root and clearExistingCaps wasn't requested), then add back
> @@ -1512,14 +1514,18 @@ virSetUIDGIDWithCaps(uid_t uid, gid_t gid, gid_t
> *groups, int ngroups,
> */
>
> if (clearExistingCaps || (uid != (uid_t)-1 && uid != 0))
> - capng_clear(CAPNG_SELECT_BOTH);
> +capng_clear(CAPNG_SELECT_BOTH);
>
> for (i = 0; i <= CAP_LAST_CAP; i++) {
> +capstr = capng_capability_to_name(i);
> +
> if (capBits & (1ULL << i)) {
> capng_update(CAPNG_ADD,
> CAPNG_EFFECTIVE|CAPNG_INHERITABLE|
> CAPNG_PERMITTED|CAPNG_BOUNDING_SET,
> i);
> +
> +VIR_DEBUG("Added '%s' to child capabilities' set", capstr);
> }
> }
>
> @@ -1579,6 +1585,27 @@ virSetUIDGIDWithCaps(uid_t uid, gid_t gid, gid_t
> *groups, int ngroups,
> goto cleanup;
> }
>
> +# ifdef PR_CAP_AMBIENT
> +/* we couldn't do this in the loop earlier above, because the
> capabilities
> + * were not applied yet, since in order to add a capability into the
> AMBIENT
> + * set, it has to be present in both the PERMITTED and INHERITABLE sets
> + * (capabilities(7))
> + */
> +for (i = 0; i <= CAP_LAST_CAP; i++) {
> +capstr = capng_capability_to_name(i);
> +
> +if (capBits & (1ULL << i)) {
> +if (prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_RAISE, i, 0, 0) < 0) {
> +virReportSystemError(errno,
> + _("prctl failed to enable '%s' in the "
> + "AMBIENT set"),
> + capstr);
> +goto cleanup;
> +}
> +}
> +}
> +# endif
This is set a bit earlier than I set it in my PoC patch, but I'll assume
it still works given the comment you added.
Reviewed-by: Daniel P. Berrangé
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 4/5] security: dac: Relabel /dev/sev in the namespace
On Thu, Jan 31, 2019 at 04:26:17PM +0100, Erik Skultety wrote: > The default permissions (0600 root:root) are of no use to the qemu > process so we need to change the owner to qemu iff running with > namespaces. > > Signed-off-by: Erik Skultety > --- > src/security/security_dac.c | 51 + > 1 file changed, 51 insertions(+) Reviewed-by: Daniel P. Berrangé 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 3/5] qemu: domain: Add /dev/sev into the domain mount namespace selectively
On Thu, Jan 31, 2019 at 04:26:16PM +0100, Erik Skultety wrote: > Instead of exposing /dev/sev to every domain, do it selectively. > > Signed-off-by: Erik Skultety > --- > src/qemu/qemu_domain.c | 24 > 1 file changed, 24 insertions(+) Reviewed-by: Daniel P. Berrangé 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 1/5] qemu: conf: Remove /dev/sev from the default cgroup device acl list
On Thu, Jan 31, 2019 at 04:26:14PM +0100, Erik Skultety wrote: > We should not give domains access to something they don't necessarily > need by default. Remove it from the qemu driver docs too. > > Signed-off-by: Erik Skultety > --- > docs/drvqemu.html.in | 2 +- > src/qemu/qemu.conf | 2 +- > src/qemu/qemu_cgroup.c | 2 +- > src/qemu/test_libvirtd_qemu.aug.in | 1 - > 4 files changed, 3 insertions(+), 4 deletions(-) Reviewed-by: Daniel P. Berrangé 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 2/5] qemu: cgroup: Expose /dev/sev/ only to domains that require SEV
On Thu, Jan 31, 2019 at 04:26:15PM +0100, Erik Skultety wrote: > SEV has a limit on number of concurrent guests. From security POV we > should only expose resources (any resources for that matter) to domains > that truly need them. > > Signed-off-by: Erik Skultety > --- > src/qemu/qemu_cgroup.c | 19 +++ > 1 file changed, 19 insertions(+) Reviewed-by: Daniel P. Berrangé 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] qemu: Rework setting process affinity
On Thu, Jan 31, 2019 at 10:34:17AM +0100, Michal Privoznik wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1503284 > > The way we currently start qemu from CPU affinity POV is as > follows: > > 1) the child process is set affinity to all online CPUs (unless > some vcpu pinning was given in the domain XML) > > 2) Once qemu is running, cpuset cgroup is configured taking > memory pinning into account > > Problem is that we let qemu allocate its memory just anywhere in > 1) and then rely in 2) to be able to move the memory to > configured NUMA nodes. This might not be always possible (e.g. > qemu might lock some parts of its memory) and is very suboptimal > (copying large memory between NUMA nodes takes significant amount > of time). > > The solution is to set affinity to one of (in priority order): > - The CPUs associated with NUMA memory affinity mask > - The CPUs associated with emulator pinning > - All online host CPUs > > Later (once QEMU has allocated its memory) we then change this > again to (again in priority order): > - The CPUs associated with emulator pinning > - The CPUs returned by numad > - The CPUs associated with vCPU pinning > - All online host CPUs > > Signed-off-by: Michal Privoznik > --- > > diff to v1 (both points suggested by Dan): > - Expanded the commit message > - fixed qemuProcessGetAllCpuAffinity so that it returns online CPU map > only > > src/qemu/qemu_process.c | 132 +++- > 1 file changed, 63 insertions(+), 69 deletions(-) Reviewed-by: Daniel P. Berrangé 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 3/4] tests: Add test for PCI usage on RISC-V
On Thu, 2019-01-31 at 17:32 +0100, Ján Tomko wrote:
> On Thu, Jan 31, 2019 at 03:34:20PM +0100, Andrea Bolognani wrote:
> > +++ b/tests/qemuxml2argvtest.c
> > @@ -3072,6 +3072,8 @@ mymain(void)
> >
> > DO_TEST("riscv64-virt",
> > QEMU_CAPS_DEVICE_VIRTIO_MMIO);
> > +DO_TEST("riscv64-virt-pci",
> > +QEMU_CAPS_OBJECT_GPEX);
>
> Consider giving DO_TEST_CAPS_ARCH_LATEST a chance, for more faithful
> viewer experience.
I would have used it, but that would have resulted in the
qemuxml2xml version being different because the macro is not
available in that test program.
Cole has taken a stab at implementing it there too a couple of
weeks ago, but ultimately shelved the attempt to avoid getting the
rest of the (non-)transitional VirtIO work blocked on it. Since I
have a few more or less related gripes about the test suite, I might
end up picking up where he left and address those as well.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] Configuring pflash devices for OVMF firmware
Paolo Bonzini writes: > On 31/01/19 13:12, Markus Armbruster wrote: >> Paolo Bonzini writes: >> >>> On 31/01/19 10:41, Markus Armbruster wrote: Paolo Bonzini writes: > On 31/01/19 09:40, Markus Armbruster wrote: >>> Maybe we should just add pflash block properties to the machine? And >>> then it can create the devices if the properties are set to a non-empty >>> value. >> What exactly do you have in mind? Something like >> >> -machine q35,ovmf-code=OVMF-CODE-NODE,ovmf-data=OVMF-DATA-NODE >> >> where OVMF-CODE-NODE and OVMF-DATA-NODE are block backend node names, >> i.e. >> >> -blockdev >> file,node-name=OVMF-CODE-NODE,read-only=on,filename=/usr/share/edk2/ovmf/OVMF_CODE.fd >> -blockdev file,node-name=OVMF-DATA-NODE,read-only=on,filename=... > > Yes, though I would call it pflash0 and pflash1. Digression... should we put traditional BIOS in flash as well? Only for new machine types, obviously. >>> >>> The blocker was that very old KVM didn't support ROMD memory regions. >>> Now on one hand we don't support those old kernel versions anymore; on >>> the other hand we have HAX and WHPX that do not support ROMD at all. >> >> This is all greek to me. I take it there's something wrong with these >> accelerators that makes (read-only?) flash memory not work, even though >> the read-only mapping we now create for traditional BIOS works. Weird, >> but I'm of course willing to take your word for it. > > Yes, as I wrote in the other message even read-only flash memory > supports commands, and these accelerators do not support direct reads + > MMIO writes on the same memory slot. Got it, thanks. > At least I checked HAX code and it doesn't; I don't know about WHPX. > >> Aside: accepting incomplete accelerators, then letting their >> incompleteness hold back things doesn't strike me as sound policy. > > Yes, there is a balance to be found between that and accepting features > from known out-of-tree forks, in order to help these out-of-tree forks > not remain forever on very old releases. I support inviting forks back into the fold, and I understand why "feature parity or go away" would be impractical there. But I'd like to see at least *commitment* to reaching feature parity. > However, another important question is---if you changed the default from > -bios to -pflash, would you also flip secure from off to on? Only TCG > and KVM supports SMM, and it's quite unlikely that the other > accelerators will support it (there are also "philosophical" debates > behind that choice...). I would not, simply because secure has always been off by default. >> Do we reject these accelerators when the user asks for firmware in >> flash? Or do we let the guest run into some more or less obscure >> failure? > > For HAX it just fails to boot I think. I'd call that a user interface bug. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] add ivshmem property master
[Please use git send-email or a normal client, this attachment is difficult to
go through, not to mention that the diff is completely broken]
On Fri, Feb 01, 2019 at 08:16:20AM +, 吴 雨霖 wrote:
[Problem Description]
I read qemu-doc.texi in qemu-mster source code, which have a explain of
migrating ivshmem below
“With device property @option{master=on}, the guest will copy the shared.memory
on migration to the destination host. With @option{master=off}, the guest will
not be able to migrate with the device attached.”
However, libvirt library can not recognize the property “master=on”. When I
directly used command "qemu-kvm -device
ivshmem-plain,id=shmem0,memdev=shmmem-shmem0,master=on,bus=pci.0,addr=0xa” to launch
a guest, qemu support ivshmem property master=on.
So, I suggest adding code to support property master=on in libvirt.
QEMU supports that, but libvirt doesn't. I remember there was a reason
for it, but I can't seem to recall what the particular reason was.
What is the reason for you to need master=on?
[Code Review]
The below is the part of source code in qemu-master. There's no definition
here about ivshmem master.
domain_conf.h
struct _virDomainShmemDef {
char *name;
unsigned long long size;
int model; /* enum virDomainShmemModel */
struct {
bool enabled;
virDomainChrSourceDef chr;
} server;
struct {
bool enabled;
unsigned vectors;
virTristateSwitch ioeventfd;
} msi;
virDomainDeviceInfo info;
};
[changed code]
src/conf/domain_conf.c
src/conf/domain_conf.h
[Detail of Source Code Modification]
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9f75dc4..b41be37 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14665,7 +14665,6 @@ virDomainShmemDefParseXML(virDomainXMLOptionPtr xmlopt,
xmlNodePtr save = ctxt->node;
xmlNodePtr server = NULL;
-
if (VIR_ALLOC(def) < 0)
return NULL;
-
if (VIR_ALLOC(def) < 0)
return NULL;
@@ -14685,12 +14684,28 @@ virDomainShmemDefParseXML(virDomainXMLOptionPtr
xmlopt,
VIR_FREE(tmp);
}
+
if (!(def->name = virXMLPropString(node, "name"))) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("shmem element must contain 'name' attribute"));
goto cleanup;
}
+
+ if ((tmp = virXMLPropString(node, "master"))) {
+ int val;
+
+ if ((val = virTristateSwitchTypeFromString(tmp)) <= 0) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("invalid ivshmem master setting
for shmem: '%s'"),
+ tmp);
+ goto cleanup;
+ }
+ def->master = val;
+ VIR_FREE(tmp);
+ }
+
+
if (virDomainParseScaledValue("./size[1]", NULL, ctxt,
>size, 1, ULLONG_MAX, false) < 0)
goto cleanup;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index f1e6e4e..615d721 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1727,8 +1727,10 @@ typedef enum {
struct _virDomainShmemDef {
char *name;
+
unsigned long long size;
int model; /* enum virDomainShmemModel */
+ virTristateSwitch master;
struct {
bool enabled;
virDomainChrSourceDef chr;
}
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] add ivshmem property master
[Problem Description]
I read qemu-doc.texi in qemu-mster source code, which have a explain of
migrating ivshmem below
“With device property @option{master=on}, the guest will copy the shared.memory
on migration to the destination host. With @option{master=off}, the guest will
not be able to migrate with the device attached.”
However, libvirt library can not recognize the property “master=on”. When I
directly used command "qemu-kvm -device
ivshmem-plain,id=shmem0,memdev=shmmem-shmem0,master=on,bus=pci.0,addr=0xa” to
launch a guest, qemu support ivshmem property master=on.
So, I suggest adding code to support property master=on in libvirt.
[Code Review]
The below is the part of source code in qemu-master. There's no definition
here about ivshmem master.
domain_conf.h
struct _virDomainShmemDef {
char *name;
unsigned long long size;
int model; /* enum virDomainShmemModel */
struct {
bool enabled;
virDomainChrSourceDef chr;
} server;
struct {
bool enabled;
unsigned vectors;
virTristateSwitch ioeventfd;
} msi;
virDomainDeviceInfo info;
};
[changed code]
src/conf/domain_conf.c
src/conf/domain_conf.h
[Detail of Source Code Modification]
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9f75dc4..b41be37 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14665,7 +14665,6 @@ virDomainShmemDefParseXML(virDomainXMLOptionPtr xmlopt,
xmlNodePtr save = ctxt->node;
xmlNodePtr server = NULL;
-
if (VIR_ALLOC(def) < 0)
return NULL;
-
if (VIR_ALLOC(def) < 0)
return NULL;
@@ -14685,12 +14684,28 @@ virDomainShmemDefParseXML(virDomainXMLOptionPtr
xmlopt,
VIR_FREE(tmp);
}
+
if (!(def->name = virXMLPropString(node, "name"))) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("shmem element must contain 'name' attribute"));
goto cleanup;
}
+
+ if ((tmp = virXMLPropString(node, "master"))) {
+ int val;
+
+ if ((val = virTristateSwitchTypeFromString(tmp)) <= 0) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("invalid ivshmem master
setting for shmem: '%s'"),
+ tmp);
+ goto cleanup;
+ }
+ def->master = val;
+ VIR_FREE(tmp);
+ }
+
+
if (virDomainParseScaledValue("./size[1]", NULL, ctxt,
>size, 1, ULLONG_MAX, false) < 0)
goto cleanup;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index f1e6e4e..615d721 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1727,8 +1727,10 @@ typedef enum {
struct _virDomainShmemDef {
char *name;
+
unsigned long long size;
int model; /* enum virDomainShmemModel */
+ virTristateSwitch master;
struct {
bool enabled;
virDomainChrSourceDef chr;
}
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [cim PATCH] Ensure nul termination of hostname
On 1/31/19 2:13 PM, Daniel P. Berrangé wrote: > Newest GCC warns that the string copying is potentially truncated and > thus not nul terminated. > > In file included from /usr/include/string.h:494, > from ../../src/Virt_HostSystem.c:23: > In function ‘strncpy’, > inlined from ‘resolve_host’ at ../../src/Virt_HostSystem.c:55:28, > inlined from ‘get_fqdn’ at ../../src/Virt_HostSystem.c:92:23, > inlined from ‘set_host_system_properties’ at > ../../src/Virt_HostSystem.c:109:13: > /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ > specified bound 256 equals destination size [-Werror=stringop-truncation] > 106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos > (__dest)); > | > ^~ > In function ‘strncpy’, > inlined from ‘resolve_host’ at ../../src/Virt_HostSystem.c:67:17, > inlined from ‘get_fqdn’ at ../../src/Virt_HostSystem.c:92:23, > inlined from ‘set_host_system_properties’ at > ../../src/Virt_HostSystem.c:109:13: > /usr/include/bits/string_fortified.h:106:10: error: ‘__builtin_strncpy’ > specified bound 256 equals destination size [-Werror=stringop-truncation] > 106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos > (__dest)); > | > ^~ > cc1: all warnings being treated as errors > > Signed-off-by: Daniel P. Berrangé > --- > src/Virt_HostSystem.c | 30 +++--- > 1 file changed, 15 insertions(+), 15 deletions(-) ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [cim PATCH] Update description of DMTF schema distribution terms
On 1/31/19 1:51 PM, Daniel P. Berrangé wrote: > The schema files that we actually download & bundle in the tar.gz dist > have removed the clause "for uses consistent with this purpose" which > is good because that clause might be considered a distribution > restriction which could make it a non-free license. > > Signed-off-by: Daniel P. Berrangé > --- > base_schema/README.DMTF | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > ACK, trivial. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] add ivshmem property master
[Problem Description]
I read qemu-doc.texi in qemu-mster source code, which have a explain of
migrating ivshmem below
“With device property @option{master=on}, the guest will copy the shared.memory
on migration to the destination host. With @option{master=off}, the guest will
not be able to migrate with the device attached.”
However, libvirt library can not recognize the property “master=on”. When I
directly used command "qemu-kvm -device
ivshmem-plain,id=shmem0,memdev=shmmem-shmem0,master=on,bus=pci.0,addr=0xa” to
launch a guest, qemu support ivshmem property master=on.
So, I suggest adding code to support property master=on in libvirt.
[Code Review]
The below is the part of source code in qemu-master. There's no definition
here about ivshmem master.
domain_conf.h
struct _virDomainShmemDef {
char *name;
unsigned long long size;
int model; /* enum virDomainShmemModel */
struct {
bool enabled;
virDomainChrSourceDef chr;
} server;
struct {
bool enabled;
unsigned vectors;
virTristateSwitch ioeventfd;
} msi;
virDomainDeviceInfo info;
};
[changed code]
src/conf/domain_conf.c
src/conf/domain_conf.h
[Detail of Source Code Modification]
src/conf/domain_conf.h
[cid:image001.png@01D4BA48.57FBFA10]
src/conf/domain_conf.c
[cid:image002.png@01D4BA48.5B8017C0]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
