Re: device compatibility interface for live migration with assigned devices

2020-07-20 Thread Jason Wang
On 2020/7/20 下午6:39, Sean Mooney wrote: On Mon, 2020-07-20 at 11:41 +0800, Jason Wang wrote: On 2020/7/18 上午12:12, Alex Williamson wrote: On Thu, 16 Jul 2020 16:32:30 +0800 Yan Zhao wrote: On Thu, Jul 16, 2020 at 12:16:26PM +0800, Jason Wang wrote: On 2020/7/14 上午7:29, Yan Zhao wrote:

Re: device compatibility interface for live migration with assigned devices

2020-07-20 Thread Yan Zhao
On Fri, Jul 17, 2020 at 10:12:58AM -0600, Alex Williamson wrote: <...> > > yes, in another reply, Alex proposed to use an interface in json format. > > I guess we can define something like > > > > { "self" : > > [ > > { "pciid" : "8086591d", > > "driver" : "i915", > >

Re: [libvirt PATCH 0/4] storage: support controlling COW attribute for pool

2020-07-20 Thread Neal Gompa
On Mon, Jul 20, 2020 at 1:33 PM Daniel P. Berrangé wrote: > > We already support a "nocow" flag for storage volumes, but this requires > extra work by the mgmt app or user when creating images on btrfs. We > want to "do the right thing" out of the box for btrfs. > > We achieve this by changint

Re: [libvirt PATCH v2 08/15] nwfilter: remove unnecessary code from ebtablesGetSubChainInsts()

2020-07-20 Thread Laine Stump
On 7/20/20 5:04 PM, Ján Tomko wrote: On a Saturday in 2020, Laine Stump wrote: On 7/15/20 11:30 AM, Ján Tomko wrote: On a Tuesday in 2020, Laine Stump wrote: Signed-off-by: Laine Stump My S-o-b stands. I still think this is the right thing to do. S-o-b merely means that you are the

Re: [PATCH 10/10] virDomainSetBlockThreshold: Mention that the event can be registered for

2020-07-20 Thread Eric Blake
On 7/15/20 8:10 AM, Peter Krempa wrote: The infrastructure supports setting the threshold also for the . Mention it in the docs. https://bugzilla.redhat.com/show_bug.cgi?id=1807741 Signed-off-by: Peter Krempa --- src/libvirt-domain.c | 3 +++ 1 file changed, 3 insertions(+) diff --git

Re: [PATCH 09/10] qemuDomainGetStorageSourceByDevstr: Look also in 'mirror' chain

2020-07-20 Thread Eric Blake
On 7/15/20 8:10 AM, Peter Krempa wrote: A disk can have a mirror, look also in it's backing chain. its (it's is only valid when you can replace with 'it is') Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 13 + 1 file changed, 9 insertions(+), 4 deletions(-)

Re: [libvirt PATCH v2 08/15] nwfilter: remove unnecessary code from ebtablesGetSubChainInsts()

2020-07-20 Thread Ján Tomko
On a Saturday in 2020, Laine Stump wrote: On 7/15/20 11:30 AM, Ján Tomko wrote: On a Tuesday in 2020, Laine Stump wrote: On failure, this function would clear out and free the list of subchains it had been called with. This is unnecessary, because the *only* caller of this function will also

Re: [PATCH 08/10] qemuDomainDiskLookupByNodename: Look also for 'mirror' node names

2020-07-20 Thread Eric Blake
On 7/15/20 8:10 AM, Peter Krempa wrote: When doing a block copy, there is another chain of images attached to a disk. Consider them as well when looking up a disk using nodename. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 8 1 file changed, 8 insertions(+)

Re: [PATCH 07/10] virStorageSourceFindByNodeName: Remove unused 'idx' argument

2020-07-20 Thread Eric Blake
On 7/15/20 8:10 AM, Peter Krempa wrote: None of the callers actually use it. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c| 9 - src/util/virstoragefile.c | 16 +++- src/util/virstoragefile.h | 3 +-- 3 files changed, 8 insertions(+), 20 deletions(-)

Re: [PATCH 06/10] qemuDomainDiskLookupByNodename: Remove unused 'idx'

2020-07-20 Thread Eric Blake
On 7/15/20 8:10 AM, Peter Krempa wrote: All callers pass NULL as the value. Remove the argument. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 12 ++-- src/qemu/qemu_domain.h | 3 +-- src/qemu/qemu_process.c | 4 ++-- 3 files changed, 5 insertions(+), 14

Re: [PATCH 05/10] virDomainSetBlockThreshold: Clarify values of @dev the event is fired for

2020-07-20 Thread Eric Blake
On 7/15/20 8:10 AM, Peter Krempa wrote: Top level image may get two events, one with the disk target (vda) and one with disk target with index (vda[3]) if the top level image has an index. Signed-off-by: Peter Krempa --- src/libvirt-domain.c | 4 1 file changed, 4 insertions(+) diff

Re: [PATCH 04/10] qemuProcessHandleBlockThreshold: Report correct indexes

2020-07-20 Thread Eric Blake
On 7/15/20 8:10 AM, Peter Krempa wrote: The index returned by qemuDomainDiskLookupByNodename is the position in the backing chain rather than the index we report in the XML. Since with -blockdev they differ now and additionally the disk source also has an index we need to fix the 'threshold'

Re: [PATCH 02/10] qemuDomainDiskBackingStoreGetName: Remove unused argument

2020-07-20 Thread Eric Blake
On 7/15/20 8:10 AM, Peter Krempa wrote: Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 1 - src/qemu/qemu_domain.h | 1 - src/qemu/qemu_process.c | 2 +- 3 files changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red

Re: [PATCH 03/10] qemuDomainDiskBackingStoreGetName: Eliminate temp variable

2020-07-20 Thread Eric Blake
On 7/15/20 8:10 AM, Peter Krempa wrote: We can return the formatted string directly. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index

[PATCH 3/5] util: log an error if virXMLNodeContentString will return NULL

2020-07-20 Thread Laine Stump
Many of our calls to xmlNodeGetContent() (which are now all via virXMLNodeContentString() are failing to check for a NULL return. We need to remedy that, but in order to make the remedy simpler, let's log an error in virXMLNodeContentString(), so that the callers don't all individually need to

[PATCH 1/5] conf: refactor virDomainBlkioDeviceParseXML to reduce calls to xmlNodeGetContent

2020-07-20 Thread Laine Stump
virDomainBlkioDeviceParseXML() calls xmlNodeGetContent() multiple times in a loop, but can easily be refactored to call it once for all element nodes, and then use the result of that one call in each of the (mutually exclusive) blocks that previously each had their own call to xmlNodeGetContent.

[FYI PATCH 5/5] util: open code virXMLNodeContentString to access the node object directly

2020-07-20 Thread Laine Stump
(I am *NOT* advocating that we apply this patch. Just providing it for informational purposes, since we had previously discussed this possibility on the list) Since it's impossible to determine whether xmlNodeContent has returned a NULL due to OOM, or due to badly formed / evil XML, this patch

[PATCH 2/5] util: replace all calls to xmlNodeGetContent with virXMLNodeContentString

2020-07-20 Thread Laine Stump
No functional change Signed-off-by: Laine Stump --- src/conf/domain_conf.c | 20 ++-- src/conf/network_conf.c | 2 +- src/conf/node_device_conf.c | 4 ++-- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c

[PATCH 0/5] be consistent about error checking xmlNodeGetContent() return

2020-07-20 Thread Laine Stump
Awhile back I noticed that calls to xmlNodeGetContent() from libvirt code were inconsistent in their handling of the returned pointer. Sometimes we would assume the return was always non-NULL (dereferencing with wild abandon without concern for the consequences), sometimes we would interpret NULL

[PATCH 4/5] treat all NULL returns from virXMLNodeContentString() as an error

2020-07-20 Thread Laine Stump
and stop erroneously equating NULL with "". The latter means that the element has empty content, while the former means there was an error during parsing (either internal with the parser, or the content of the XML was bad). Signed-off-by: Laine Stump --- src/conf/domain_conf.c | 68

Re: [PATCH 01/10] virDomainSetBlockThreshold: Document values of '@dev' better

2020-07-20 Thread Eric Blake
On 7/15/20 8:10 AM, Peter Krempa wrote: Mention where to obtain the index and how it's treated. Signed-off-by: Peter Krempa --- src/libvirt-domain.c | 6 ++ 1 file changed, 6 insertions(+) Reviewed-by: Eric Blake diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index

Re: [PATCH 5/5] qemuDomainBlockPivot: Ignore failures of creating active layer bitmap

2020-07-20 Thread Eric Blake
On 7/16/20 9:20 AM, Peter Krempa wrote: Ignore errors from creating "libvirt-tmp-activewrite" bitmap. This prevents failures of finishing blockjobs if the bitmap already exists. Note that if the bitmap exists, the worst case that can happen is that more bits are marked as dirty in the resulting

Re: [PATCH 4/5] qemuDomainBlockPivot: Rename 'actions' to 'bitmapactions'

2020-07-20 Thread Eric Blake
On 7/16/20 9:20 AM, Peter Krempa wrote: There are two possible 'transaction' command arguments in the function. Rename 'actions' as they deal with creating bitmaps only. Signed-off-by: Peter Krempa --- src/qemu/qemu_driver.c | 14 +++--- 1 file changed, 7 insertions(+), 7

Re: [PATCH 1/5] qemu: blockjob: Don't base bitmap handling of active-layer block commit on QEMU_CAPS_BLOCKDEV_REOPEN

2020-07-20 Thread Eric Blake
On 7/16/20 9:20 AM, Peter Krempa wrote: The handler finalizing the active layer block commit doesn't actually reopen the file for active layer block commit, so the comment and check are invalid. Signed-off-by: Peter Krempa --- src/qemu/qemu_blockjob.c | 3 ++- src/qemu/qemu_driver.c | 6

Re: [PATCH 2/5] qemu: blockjob: Actually delete temporary bitmap on failed active commit

2020-07-20 Thread Eric Blake
On 7/16/20 9:20 AM, Peter Krempa wrote: Commit 20a7abc2d2d tried to delete the possibly leftover bitmap but neglected to call the actual monitor to do so. Signed-off-by: Peter Krempa --- src/qemu/qemu_blockjob.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) Yeah,

Re: [PATCH 3/5] qemu: block: Remove 'active-write' bitmap even if there are no bitmaps to merge

2020-07-20 Thread Eric Blake
On 7/16/20 9:20 AM, Peter Krempa wrote: The 'libvirt-tmp-activewrite' bitmap is added during the 'pivot' operation of block copy and active layer block commit operations regardless of whether there are any bitmaps to merge, but was not removed unless a bitmap was merged. This meant that

Re: Setting 'nodatacow' on VM image when on Btrfs

2020-07-20 Thread Chris Murphy
On Mon, Jul 20, 2020 at 12:11 PM Daniel P. Berrangé wrote: > > On Mon, Jul 13, 2020 at 08:06:22AM -0600, Chris Murphy wrote: > > On Mon, Jul 13, 2020 at 6:20 AM Daniel P. Berrangé > > wrote: > > > > > > On Sat, Jul 11, 2020 at 07:28:43PM -0600, Chris Murphy wrote: > > > > > > Also, the location

Re: [libvirt PATCH 9/9] rpc: use new virt-nc binary for remote tunnelling

2020-07-20 Thread Daniel P . Berrangé
On Mon, Jul 20, 2020 at 08:20:12PM +0200, Andrea Bolognani wrote: > On Mon, 2020-07-20 at 18:21 +0100, Daniel P. Berrangé wrote: > > On Mon, Jul 20, 2020 at 07:18:55PM +0200, Andrea Bolognani wrote: > > > The other day I randomly realized the ssh-based transports already > > > accept a 'netcat'

Re: [libvirt PATCH 9/9] rpc: use new virt-nc binary for remote tunnelling

2020-07-20 Thread Andrea Bolognani
On Mon, 2020-07-20 at 18:21 +0100, Daniel P. Berrangé wrote: > On Mon, Jul 20, 2020 at 07:18:55PM +0200, Andrea Bolognani wrote: > > The other day I randomly realized the ssh-based transports already > > accept a 'netcat' URI parameter which can be used to point libvirt > > to a non-standard nc

Re: Setting 'nodatacow' on VM image when on Btrfs

2020-07-20 Thread Daniel P . Berrangé
On Mon, Jul 13, 2020 at 08:06:22AM -0600, Chris Murphy wrote: > On Mon, Jul 13, 2020 at 6:20 AM Daniel P. Berrangé > wrote: > > > > On Sat, Jul 11, 2020 at 07:28:43PM -0600, Chris Murphy wrote: > > > > Also, the location for GNOME Boxes doesn't exist at install time, so > > > the installer

[libvirt PATCH 1/4] util: add a helper method for controlling the COW flag on btrfs

2020-07-20 Thread Daniel P . Berrangé
btrfs defaults to performing copy-on-write for files. This is often undesirable for VM images, so we need to be able to control whether this behaviour is used. The virFileSetCOW() will allow for this. We use a tristate, since out of the box, we want the default behaviour attempt to disable cow,

[libvirt PATCH 4/4] conf: add control over COW for storage pool directories

2020-07-20 Thread Daniel P . Berrangé
The storage pool code now attempts to disable COW by default on btrfs, but management applications may wish to override this behaviour. Thus we introduce a concept of storage pool features: If the feature policy is set, it will be enforced. It will always return an hard error if COW

[libvirt PATCH 3/4] storage: attempt to disable COW by default

2020-07-20 Thread Daniel P . Berrangé
This calls virFileSetCOW when building a pool with a request to attempt, but not require, COW to be disabled. The effect is that nothing changes on non-btrfs filesystems, but btrfs will get COW disabled on the directory. This setting is then inherited by all newly created files in the pool,

[libvirt PATCH 2/4] storage: convert to use virFileSetCOW

2020-07-20 Thread Daniel P . Berrangé
When disabling COW on individual files, we now use the virFileSetCOW method. Note that this change has a slight semantic difference to the old implementation. The original code reported errors but returned success when disabling COW failed. With this new code, we will always report an error if

[libvirt PATCH 0/4] storage: support controlling COW attribute for pool

2020-07-20 Thread Daniel P . Berrangé
We already support a "nocow" flag for storage volumes, but this requires extra work by the mgmt app or user when creating images on btrfs. We want to "do the right thing" out of the box for btrfs. We achieve this by changint the storage pool code so that when creating a storage pool we always try

Re: [libvirt PATCH] docs: virConnectGetCapabilities do not provide pool types

2020-07-20 Thread John Ferlan
On 7/20/20 3:50 AM, Pino Toscano wrote: > Remove the paragraph in the storage pool page that mentions > virConnectGetCapabilities, as virConnectGetCapabilities does not return > any information about pools. > > Signed-off-by: Pino Toscano > --- > docs/formatstoragecaps.html.in | 6 -- >

Re: [libvirt PATCH 9/9] rpc: use new virt-nc binary for remote tunnelling

2020-07-20 Thread Daniel P . Berrangé
On Mon, Jul 20, 2020 at 07:18:55PM +0200, Andrea Bolognani wrote: > On Wed, 2020-07-15 at 16:11 +0200, Andrea Bolognani wrote: > > On Wed, 2020-07-15 at 14:25 +0100, Daniel P. Berrangé wrote: > > > On Wed, Jul 15, 2020 at 02:25:14PM +0200, Andrea Bolognani wrote: > > > > Mh, that makes sense but

Re: [libvirt PATCH 9/9] rpc: use new virt-nc binary for remote tunnelling

2020-07-20 Thread Andrea Bolognani
On Wed, 2020-07-15 at 16:11 +0200, Andrea Bolognani wrote: > On Wed, 2020-07-15 at 14:25 +0100, Daniel P. Berrangé wrote: > > On Wed, Jul 15, 2020 at 02:25:14PM +0200, Andrea Bolognani wrote: > > > Mh, that makes sense but I'm still wary of using "proxy" due to the > > > potential for confusion,

[PATCH 1/1] formatdomain.html.in: mention pSeries NVDIMM 'align down' mechanic

2020-07-20 Thread Daniel Henrique Barboza
The reason why we align down the guest area (total-size - label-size) is explained in the body of qemuDomainNVDimmAlignSizePseries(). This behavior must also be documented in the user docs. Signed-off-by: Daniel Henrique Barboza --- docs/formatdomain.html.in | 6 -- 1 file changed, 4

[libvirt PATCH 3/3] Add a check attribute on the mac address element

2020-07-20 Thread Daniel P . Berrangé
From: Bastien Orivel This is only used in the ESX driver where, when set to "no", it will ignore all the checks libvirt does about the origin of the MAC address (whether or not it's in a VMWare OUI) and forward the original one to the ESX server telling it not to check it either. This allows

[libvirt PATCH 2/3] vmx: support outputing the type attribute for MAC addresses

2020-07-20 Thread Daniel P . Berrangé
When support for MAC addresses having a type='static|generated' attribute was added in: commit 454e5961abf40c14f8b6d7ee216229e68fd170bf Author: Bastien Orivel Date: Mon Jul 13 16:28:53 2020 +0200 Add a type attribute on the mac address element the VMX -> XML parser was not updated.

[libvirt PATCH 0/3] vmx: fix handling of VMX MAC address options

2020-07-20 Thread Daniel P . Berrangé
We first had a proposal to add a "check" attribute for MAC addresses https://www.redhat.com/archives/libvir-list/2020-July/msg00617.html For reasons I don't understand this was then replaced by a "type" attribute https://www.redhat.com/archives/libvir-list/2020-July/msg00656.html with the

[libvirt PATCH 1/3] vmx: fix logic handling mac address type

2020-07-20 Thread Daniel P . Berrangé
With the current formatter, the XML snippets: result in ethernet1.present = "true" ethernet1.networkName = "br1" ethernet1.connectionType = "bridged" ethernet1.addressType = "static" ethernet1.address = "00:0c:29:dd:ee:fe"

Re: [libvirt PATCH] network: split out networkSetIPv6Sysctl

2020-07-20 Thread Andrea Bolognani
On Wed, 2020-07-15 at 23:27 +0200, Ján Tomko wrote: > Refactor networkSetIPv6Sysctls to remove repetition and reuse > of the 'field' variable. > > Signed-off-by: Ján Tomko > --- > src/network/bridge_driver.c | 71 ++--- > 1 file changed, 35 insertions(+), 36

Re: The issue about adding multipath device's targets into qemu-pr-helper's namespace

2020-07-20 Thread Lin Ma
On 2020-07-17 07:05, Michal Prívozník wrote: On 7/15/20 5:35 AM, Lin Ma wrote: On 2020-07-14 14:38, Michal Privoznik wrote: On 7/14/20 3:41 PM, Lin Ma wrote: Hi all, I have a namespace question about passthrough disk(multipath device). In case of enabling namespace and cgroups in qemu.conf,

Re: [GSoC][PATCH v5] qemu_domainjob: introduce `privateData` for `qemuDomainJob`

2020-07-20 Thread Michal Privoznik
On 7/16/20 1:48 PM, Prathamesh Chavan wrote: To remove dependecy of `qemuDomainJob` on job specific paramters, a `privateData` pointer is introduced. To handle it, structure of callback functions is also introduced. Signed-off-by: Prathamesh Chavan --- Previous version of this patch can be

Re: [RFC v2 1/1] memory: Delete assertion in memory_region_unregister_iommu_notifier

2020-07-20 Thread Peter Xu
On Mon, Jul 20, 2020 at 12:02:06PM +0800, Jason Wang wrote: > Right, so there's no need to deal with unmap in vtd's replay implementation > (as what generic one did). We don't even for now; see vtd_page_walk_info.notify_unmap. Thanks, -- Peter Xu

Re: [PATCH] qemu_driver.c: Fix domfsinfo for non-PCI device information from guest agent

2020-07-20 Thread Thomas Huth
On 20/07/2020 12.32, Daniel P. Berrangé wrote: > On Mon, Jul 20, 2020 at 12:22:33PM +0200, Thomas Huth wrote: >> qemuAgentFSInfoToPublic() currently only sets the devAlias for PCI devices. >> However, the QEMU guest agent could also provide the device name in the >> "dev" field of the response for

Re: device compatibility interface for live migration with assigned devices

2020-07-20 Thread Sean Mooney
On Mon, 2020-07-20 at 11:41 +0800, Jason Wang wrote: > On 2020/7/18 上午12:12, Alex Williamson wrote: > > On Thu, 16 Jul 2020 16:32:30 +0800 > > Yan Zhao wrote: > > > > > On Thu, Jul 16, 2020 at 12:16:26PM +0800, Jason Wang wrote: > > > > On 2020/7/14 上午7:29, Yan Zhao wrote: > > > > > hi folks, >

Re: [PATCH] qemu_driver.c: Fix domfsinfo for non-PCI device information from guest agent

2020-07-20 Thread Daniel P . Berrangé
On Mon, Jul 20, 2020 at 12:22:33PM +0200, Thomas Huth wrote: > qemuAgentFSInfoToPublic() currently only sets the devAlias for PCI devices. > However, the QEMU guest agent could also provide the device name in the > "dev" field of the response for other devices instead (well, at least after >

[PATCH] qemu_driver.c: Fix domfsinfo for non-PCI device information from guest agent

2020-07-20 Thread Thomas Huth
qemuAgentFSInfoToPublic() currently only sets the devAlias for PCI devices. However, the QEMU guest agent could also provide the device name in the "dev" field of the response for other devices instead (well, at least after fixing another problem in the current QEMU guest agent...). So if creating

Re: [libvirt PATCH] spec: Drop explicit dependency on ncurses

2020-07-20 Thread Erik Skultety
On Sun, Jul 19, 2020 at 12:54:43AM +0200, Andrea Bolognani wrote: > We don't actually use ncurses directly: readline needs it, but > that's a readline implementation detail and not something that we > should concern ourselves with. > > Signed-off-by: Andrea Bolognani > --- > Test pipeline where

[PATCH] qemu: pre-create the dbus directory in qemuStateInitialize

2020-07-20 Thread Bihong Yu
>From 187323ce736dcd9b1a177d552630b0c6859a4798 Mon Sep 17 00:00:00 2001 From: Bihong Yu Date: Tue, 14 Jul 2020 15:44:05 +0800 Subject: [PATCH] qemu: pre-create the dbus directory in qemuStateInitialize There are races condiction to make '/run/libvirt/qemu/dbus' directory in virDirCreateNoFork()

[libvirt PATCH] docs: virConnectGetCapabilities do not provide pool types

2020-07-20 Thread Pino Toscano
Remove the paragraph in the storage pool page that mentions virConnectGetCapabilities, as virConnectGetCapabilities does not return any information about pools. Signed-off-by: Pino Toscano --- docs/formatstoragecaps.html.in | 6 -- 1 file changed, 6 deletions(-) diff --git

Re: [libvirt PATCH] spec: Don't require mdevctl on RHEL 7

2020-07-20 Thread Erik Skultety
On Sun, Jul 19, 2020 at 12:46:05AM +0200, Andrea Bolognani wrote: > mdevctl is a relatively new tool that's packaged for Fedora and > RHEL 8, but not for RHEL 7. Make the dependency conditional to > avoid the libvirt-daemon-driver-nodedev package becoming > uninstallable on that platform. > >