Re: [PATCH 0/4] jobs: make & use generalized virDomainObjInitJob()

On Thu, Jul 21, 2022 at 01:54:45PM +0200, Kristina Hanicova wrote: This series uses qemuDomainObjInitJob() as virDomainObjInitJob() in all I would prefer to rather use a name "virDomainJobObjInit" since it initialises the job obj. It is not a domain object that initialises a job. With that

Re: [PATCH v2 8/8] domain_conf: rewrite variable setting

On Thu, Jul 21, 2022 at 12:45:53PM +0200, Kristina Hanicova wrote: Signed-off-by: Kristina Hanicova Reviewed-by: Martin Kletzander --- src/conf/domain_conf.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index

Re: [PATCH v2 7/8] domain_conf: rewrite conditions in virDomainObjWaitUntil()

On Thu, Jul 21, 2022 at 12:45:52PM +0200, Kristina Hanicova wrote: This patch rewrites conditions to make the code easier and less structured. "less structured" bears a negative connotation, but I must say the code is very easy now =D ;) s/easier/easier to read/; s/structured/nested/

Re: [libvirt][PATCH v13 0/6] Support query and use SGX

On Thu, Jul 21, 2022 at 05:09:15PM +0200, Michal Prívozník wrote: > On 7/21/22 16:29, Daniel P. Berrangé wrote: > > On Thu, Jul 21, 2022 at 04:10:11PM +0200, Michal Prívozník wrote: > >> On 7/21/22 15:24, Daniel P. Berrangé wrote: > >>> On Thu, Jul 21, 2022 at 03:12:05PM +0200, Michal Prívozník

Re: [PATCH v2 6/8] domain_conf: use early return in virDomainObjAssignDef()

On Thu, Jul 21, 2022 at 12:45:51PM +0200, Kristina Hanicova wrote: Signed-off-by: Kristina Hanicova Reviewed-by: Martin Kletzander signature.asc Description: PGP signature

Re: [libvirt][PATCH v13 0/6] Support query and use SGX

On 7/21/22 16:29, Daniel P. Berrangé wrote: > On Thu, Jul 21, 2022 at 04:10:11PM +0200, Michal Prívozník wrote: >> On 7/21/22 15:24, Daniel P. Berrangé wrote: >>> On Thu, Jul 21, 2022 at 03:12:05PM +0200, Michal Prívozník wrote: On 7/21/22 10:06, Daniel P. Berrangé wrote: Agreed. While

Re: [PATCH v2 5/8] domain_conf: extend switch with if in virDomainChrDefFree()

On Thu, Jul 21, 2022 at 05:04:43PM +0200, Kristina Hanicova wrote: sorry for the confusing commit message, it should have been "domain_conf: extend switch in virDomainChrDefFree()" only:D ok, with that and ... Kristina On Thu, Jul 21, 2022 at 12:46 PM Kristina Hanicova wrote: Switch is

Re: [PATCH v2 5/8] domain_conf: extend switch with if in virDomainChrDefFree()

sorry for the confusing commit message, it should have been "domain_conf: extend switch in virDomainChrDefFree()" only:D Kristina On Thu, Jul 21, 2022 at 12:46 PM Kristina Hanicova wrote: > Switch is used for just one case, but a more future proof > approach is to handle all enum values. > >

Re: [PATCH v2 4/8] domain_conf: remove breaks after return in virDomainChrSourceDefIsEqual()

On Thu, Jul 21, 2022 at 12:45:49PM +0200, Kristina Hanicova wrote: Signed-off-by: Kristina Hanicova Reviewed-by: Martin Kletzander signature.asc Description: PGP signature

Re: [PATCH v2 3/8] domain_capabilities: reformat virDomainCapsCPUCustomFormat()

On Thu, Jul 21, 2022 at 12:45:48PM +0200, Kristina Hanicova wrote: Signed-off-by: Kristina Hanicova I was thinking if this would look nicer with virXMLFormatElement, but then I realised it's just a two lines of code and it does not matter at all =) Reviewed-by: Martin Kletzander ---

Re: [libvirt][PATCH v13 0/6] Support query and use SGX

On Thu, Jul 21, 2022 at 04:10:11PM +0200, Michal Prívozník wrote: > On 7/21/22 15:24, Daniel P. Berrangé wrote: > > On Thu, Jul 21, 2022 at 03:12:05PM +0200, Michal Prívozník wrote: > >> On 7/21/22 10:06, Daniel P. Berrangé wrote: > >> Agreed. While libvirt can allow /dev/sgx* in CGroups (we do

Re: [PATCH v2 2/8] domain_capabilities: reformat virDomainCapsFeatureSEVFormat()

On Thu, Jul 21, 2022 at 12:45:47PM +0200, Kristina Hanicova wrote: Signed-off-by: Kristina Hanicova Reviewed-by: Martin Kletzander signature.asc Description: PGP signature

Re: [PATCH v2 1/8] domain_capabilities: use early return in virDomainCapsFeatureSEVFormat()

On Thu, Jul 21, 2022 at 12:45:46PM +0200, Kristina Hanicova wrote: Signed-off-by: Kristina Hanicova Reviewed-by: Martin Kletzander signature.asc Description: PGP signature

Re: [libvirt][PATCH v13 0/6] Support query and use SGX

On 7/21/22 15:24, Daniel P. Berrangé wrote: > On Thu, Jul 21, 2022 at 03:12:05PM +0200, Michal Prívozník wrote: >> On 7/21/22 10:06, Daniel P. Berrangé wrote: >>> On Wed, Jul 20, 2022 at 11:12:56PM +, Yang, Lin A wrote: > This version is a bit better than the previous one. But we're at

Re: [libvirt][PATCH v13 0/6] Support query and use SGX

On Thu, Jul 21, 2022 at 03:12:05PM +0200, Michal Prívozník wrote: > On 7/21/22 10:06, Daniel P. Berrangé wrote: > > On Wed, Jul 20, 2022 at 11:12:56PM +, Yang, Lin A wrote: > >>> This version is a bit better than the previous one. But we're at version > >>> 13 and I am still unable to even

Re: [libvirt PATCH] qemu_migration: Use EnterMonitorAsync in qemuDomainGetMigrationBlockers

On 7/21/22 15:05, Jiri Denemark wrote: > The code is run with an async job and thus needs to make sure a nested > job is acquired before entering the monitor. > > While touching the code in qemuMigrationSrcIsAllowed I also fixed the > grammar which was accidentally broken by

Re: [libvirt][PATCH v13 0/6] Support query and use SGX

On 7/21/22 10:06, Daniel P. Berrangé wrote: > On Wed, Jul 20, 2022 at 11:12:56PM +, Yang, Lin A wrote: >>> This version is a bit better than the previous one. But we're at version >>> 13 and I am still unable to even start a guest. Please, make sure that this >>> basic functionality works in

[libvirt PATCH] qemu_migration: Use EnterMonitorAsync in qemuDomainGetMigrationBlockers

The code is run with an async job and thus needs to make sure a nested job is acquired before entering the monitor. While touching the code in qemuMigrationSrcIsAllowed I also fixed the grammar which was accidentally broken by v8.5.0-140-g2103807e33. Signed-off-by: Jiri Denemark ---

Re: [PATCH v2 1/2] domain_conf: remove else after return / goto

On Thu, Jul 21, 2022 at 01:30:44PM +0200, Kristina Hanicova wrote: The else branches are redundant because the execution will never reach them if the conditions in the previous 'if' branches are true. I think this looks cleaner and is more readable, because having 'else' branch indicates that

Re: [PATCH v2 2/2] domain_conf: rewrite if else functions to switch

On Thu, Jul 21, 2022 at 01:30:45PM +0200, Kristina Hanicova wrote: Pattern of using switch instead of a long if else construction is used everywhere, so I used it here as well to make the code more consistent (and remove that else after return). I also included all the values from the enum.

Re: [PATCH 2/2] qemu_cgroup: Introduce qemuCgroupAllowDevicesPaths()

On Thu, Jul 21, 2022 at 12:45:29PM +0200, Michal Prívozník wrote: On 7/21/22 12:31, Michal Privoznik wrote: We have qemuCgroupAllowDevicePath() which sets up devices controller for just one path. And if we have more paths we have to call it in a loop. So far, we have just one such place, but

Re: [PATCH 1/2] qemu_cgroup: Avoid ternary operator when setting @deviceACL

On Thu, Jul 21, 2022 at 12:31:41PM +0200, Michal Privoznik wrote: Inside of the qemuSetupDevicesCgroup() there's @deviceACL variable, which points to a string list of devices that are allowed in devices controller by default. This list can either come from qemu.conf (cfg->cgroupDeviceACL) or

[PATCH 0/4] jobs: make & use generalized virDomainObjInitJob()

This series uses qemuDomainObjInitJob() as virDomainObjInitJob() in all drivers that use virDomainObjJob structure. Kristina Hanicova (4): qemu & hypervisor: move qemuDomainObjInitJob() into hypervisor libxl: use virDomainObjInitJob() LXC: use virDomainObjInitJob() CH: use

[PATCH 3/4] LXC: use virDomainObjInitJob()

This patch removes and replaces virLXCDomainObjInitJob() with general virDomainObjInitJob(). Signed-off-by: Kristina Hanicova --- src/lxc/lxc_domain.c | 13 + 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c index

[PATCH 4/4] CH: use virDomainObjInitJob()

This patch removes and replaces virCHDomainObjInitJob() with general virDomainObjInitJob(). Signed-off-by: Kristina Hanicova --- src/ch/ch_domain.c | 13 + 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c index

[PATCH 1/4] qemu & hypervisor: move qemuDomainObjInitJob() into hypervisor

This patch moves qemuDomainObjInitJob() as virDomainObjInitJob() into hypervisor in order to be used by other drivers as well. Signed-off-by: Kristina Hanicova --- src/hypervisor/domain_job.c | 25 + src/hypervisor/domain_job.h | 4 src/libvirt_private.syms| 1

[PATCH 2/4] libxl: use virDomainObjInitJob()

This patch removes and replaces libxlDomainObjInitJob() with general virDomainObjInitJob(). Signed-off-by: Kristina Hanicova --- src/libxl/libxl_domain.c | 17 +++-- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c

[PATCH v2 0/2] domain_conf: rewriting else after return

diff to v1: * split one commit into two * rewrite the functions in the second one to use the switch (suggested by Daniel, thanks) v1: https://listman.redhat.com/archives/libvir-list/2022-July/232989.html Kristina Hanicova (2): domain_conf: remove else after return / goto domain_conf:

[PATCH v2 2/2] domain_conf: rewrite if else functions to switch

Pattern of using switch instead of a long if else construction is used everywhere, so I used it here as well to make the code more consistent (and remove that else after return). I also included all the values from the enum. Signed-off-by: Kristina Hanicova --- src/conf/domain_conf.c | 42

[PATCH v2 1/2] domain_conf: remove else after return / goto

The else branches are redundant because the execution will never reach them if the conditions in the previous 'if' branches are true. I think this looks cleaner and is more readable, because having 'else' branch indicates that no return / break / goto is in the previous branch and the function

[PATCH v2 5/8] domain_conf: extend switch with if in virDomainChrDefFree()

Switch is used for just one case, but a more future proof approach is to handle all enum values. Signed-off-by: Kristina Hanicova --- src/conf/domain_conf.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index

[PATCH v2 7/8] domain_conf: rewrite conditions in virDomainObjWaitUntil()

This patch rewrites conditions to make the code easier and less structured. Signed-off-by: Kristina Hanicova --- src/conf/domain_conf.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index

[PATCH v2 2/8] domain_capabilities: reformat virDomainCapsFeatureSEVFormat()

Signed-off-by: Kristina Hanicova --- src/conf/domain_capabilities.c | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c index bb36a956db..6e8f957657 100644 --- a/src/conf/domain_capabilities.c +++

[PATCH v2 8/8] domain_conf: rewrite variable setting

Signed-off-by: Kristina Hanicova --- src/conf/domain_conf.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bbe6574487..d3872830c9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4216,12

[PATCH v2 6/8] domain_conf: use early return in virDomainObjAssignDef()

Signed-off-by: Kristina Hanicova --- src/conf/domain_conf.c | 32 +--- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 41eb105a6c..507bff953c 100644 --- a/src/conf/domain_conf.c +++

[PATCH v2 4/8] domain_conf: remove breaks after return in virDomainChrSourceDefIsEqual()

Signed-off-by: Kristina Hanicova --- src/conf/domain_conf.c | 13 + 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 26a241db38..b903dac1cb 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2844,21

[PATCH v2 3/8] domain_capabilities: reformat virDomainCapsCPUCustomFormat()

Signed-off-by: Kristina Hanicova --- src/conf/domain_capabilities.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c index 6e8f957657..653123f293 100644 --- a/src/conf/domain_capabilities.c +++

[PATCH v2 0/8] domain_conf & domain_capabilities: small refactoring

diff to v1: * dropped 1 wrong commit (thanks Daniel) * improved setting of the variable (suggested by Peter, Michal, Martin) * extended the switch to include all cases instead of simple if to be future proof (suggested by Peter) v1:

[PATCH v2 1/8] domain_capabilities: use early return in virDomainCapsFeatureSEVFormat()

Signed-off-by: Kristina Hanicova --- src/conf/domain_capabilities.c | 33 - 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c index 33570a51db..bb36a956db 100644 ---

Re: [PATCH 2/2] qemu_cgroup: Introduce qemuCgroupAllowDevicesPaths()

On 7/21/22 12:31, Michal Privoznik wrote: > We have qemuCgroupAllowDevicePath() which sets up devices > controller for just one path. And if we have more paths we have > to call it in a loop. So far, we have just one such place, but > soon we'll have another one (for SGX memory). Separate the loop

[PATCH 2/2] qemu_cgroup: Introduce qemuCgroupAllowDevicesPaths()

We have qemuCgroupAllowDevicePath() which sets up devices controller for just one path. And if we have more paths we have to call it in a loop. So far, we have just one such place, but soon we'll have another one (for SGX memory). Separate the loop into its own function so that it can be reused.

[PATCH 1/2] qemu_cgroup: Avoid ternary operator when setting @deviceACL

Inside of the qemuSetupDevicesCgroup() there's @deviceACL variable, which points to a string list of devices that are allowed in devices controller by default. This list can either come from qemu.conf (cfg->cgroupDeviceACL) or from a builtin @defaultDeviceACL. However, a multiline ternary operator

[PATCH 0/2] qemu_cgroup: Two almost trivial cleanups

I'm trying to fix SGX patches I reviewed earlier [1] and these would simplify my attempts. 1: https://listman.redhat.com/archives/libvir-list/2022-July/232679.html Michal Prívozník (2): qemu_cgroup: Avoid ternary operator when setting @deviceACL qemu_cgroup: Introduce

[libvirt PATCH 2/3] qemu: Assign default alias to IOMMU devices

Signed-off-by: Andrea Bolognani --- src/qemu/qemu_alias.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index 7efd91051e..7b91fe3141 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -597,6 +597,14 @@

[libvirt PATCH 3/3] qemu: Add IOMMU device alias to command line

Note that we can only do this for intel-iommu and virtio-iommu, which are configured using -device; smmuv3 is configured using a machine type property, so there's no room on the command line for an alias in that case. https://bugzilla.redhat.com/show_bug.cgi?id=2108483 Signed-off-by: Andrea

[libvirt PATCH 1/3] schema: Allow IOMMU devices to have aliases

Signed-off-by: Andrea Bolognani --- src/conf/schemas/domaincommon.rng | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 2f07c25430..d15dd33f47 100644 --- a/src/conf/schemas/domaincommon.rng +++

[libvirt PATCH 0/3] qemu: Allow IOMMU devices to have aliases

Andrea Bolognani (3): schema: Allow IOMMU devices to have aliases qemu: Assign default alias to IOMMU devices qemu: Add IOMMU device alias to command line src/conf/schemas/domaincommon.rng | 3 +++ src/qemu/qemu_alias.c | 10

Re: [PATCH 0/2] A couple of migration blocker query patches

On Thu, Jul 21, 2022 at 02:21:11 -0400, Laine Stump wrote: > Five minutes after pushing eperezma's patches that query QEMU for > migration blockers, I realized that the query would be called during > "offline migration" (when there is no QEMU process running). So the > 1st patch moves the query

Re: [PATCH 9/9] domain_conf: rewrite variable setting to ternary operator

On Wed, Jul 20, 2022 at 04:28:30PM +0200, Michal Prívozník wrote: On 7/20/22 15:40, Peter Krempa wrote: On Wed, Jul 20, 2022 at 15:11:12 +0200, Kristina Hanicova wrote: Signed-off-by: Kristina Hanicova --- src/conf/domain_conf.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-)

Re: [libvirt][PATCH v13 0/6] Support query and use SGX

On Wed, Jul 20, 2022 at 11:12:56PM +, Yang, Lin A wrote: > > This version is a bit better than the previous one. But we're at version > > 13 and I am still unable to even start a guest. Please, make sure that this > > basic functionality works in v14, because this is plain waste of precious >

[PATCH 0/2] A couple of migration blocker query patches

Five minutes after pushing eperezma's patches that query QEMU for migration blockers, I realized that the query would be called during "offline migration" (when there is no QEMU process running). So the 1st patch moves the query down so it will only be called during live migration. The 2nd patch

[PATCH 1/2] qemu: don't try to query QEMU about migration blockers during offline migration

The new code that queries QEMU about migration blockers was put at the top of qemuMigrationSrcIsAllowed(), but that function can also be called in the case of offline migration (ie when the domain is inactive / QEMU isn't running). This check should have been put inside the "if (!(flags &

[PATCH 2/2] qemu: skip hardcoded hostdev migration check if QEMU can do it for us

libvirt currently will block migration for any vfio-assigned device unless it is a network device that is associated with a virtio-net failover device (ie. if the hostdev object has a teaming->type == VIR_DOMAIN_NET_TEAMING_TYPE_TRANSIENT). In the future there will be other vfio devices that can