Re: [PATCH v1 01/12] update-linux-headers: Add asm-riscv/kvm.h

2021-11-22 Thread Alistair Francis
On Sat, Nov 20, 2021 at 5:51 PM Yifei Jiang wrote: > > Add asm-riscv/kvm.h for RISC-V KVM, and update linux/kvm.h > > Signed-off-by: Yifei Jiang > Signed-off-by: Mingwang Li Acked-by: Alistair Francis Alistair > --- > linux-headers/asm-riscv/kvm.h | 128 ++ >

Re: [PATCH 09/12] virSecurityDeviceLabelDefParseXML: Use automatic memory clearing for temp strings

2021-11-22 Thread Tim Wiederhake
On Mon, 2021-11-22 at 18:12 +0100, Peter Krempa wrote: > Apart from code simplification the refactor of 'model' fixes an > unlikely > memory leak of the string if a duplicate model is found. > > While the coversion of 'label' variable may seem unnecessary it will > come in handy in the next patch.

Re: [PATCH 08/12] virSecurityLabelDefParseXML: Don't use 'virXPathStringLimit'

2021-11-22 Thread Tim Wiederhake
On Mon, 2021-11-22 at 18:12 +0100, Peter Krempa wrote: > virXPathStringLimit doesn't give callers a way to differentiate > between > the queried XPath being empty and the length limit being exceeded. > > This means that callers are either overwriting the error message or > ignoring it altogether.

Re: [PATCH 04/12] virSecurityLabelDefParseXML: Don't reuse temporary string 'p'

2021-11-22 Thread Tim Wiederhake
On Mon, 2021-11-22 at 18:12 +0100, Peter Krempa wrote: > (...) > diff --git a/src/util/virseclabel.h b/src/util/virseclabel.h > index eca4d09d24..7e62f8a2e2 100644 > --- a/src/util/virseclabel.h > +++ b/src/util/virseclabel.h > @@ -43,6 +43,7 @@ struct _virSecurityLabelDef { >  }; > > > + >  /* S

Re: [PATCH 09/12] virSecurityDeviceLabelDefParseXML: Use automatic memory clearing for temp strings

2021-11-22 Thread Peter Krempa
On Mon, Nov 22, 2021 at 18:12:29 +0100, Peter Krempa wrote: > Apart from code simplification the refactor of 'model' fixes an unlikely > memory leak of the string if a duplicate model is found. > > While the coversion of 'label' variable may seem unnecessary it will > come in handy in the next pat

[PATCH 07/12] virNodeDeviceCapVPDParseCustomFields: Don't use 'virXPathStringLimit'

2021-11-22 Thread Peter Krempa
virXPathStringLimit doesn't give callers a way to differentiate between the queried XPath being empty and the length limit being exceeded. This means that callers are overwriting the error message. Move the length checks into the caller. Signed-off-by: Peter Krempa --- src/conf/node_device_con

[PATCH 06/12] virSecurityLabelDefParseXML: Remove pointless 'error' label

2021-11-22 Thread Peter Krempa
Signed-off-by: Peter Krempa --- src/conf/domain_conf.c | 19 --- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 99bee98df8..ee44bbbd4b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7808,7

[PATCH 10/12] virSecurityDeviceLabelDefParseXML: Don't use 'virXPathStringLimit'

2021-11-22 Thread Peter Krempa
virXPathStringLimit doesn't give callers a way to differentiate between the queried XPath being empty and the length limit being exceeded. This means that the callers is completely ignoring the error. Move the length check into the caller. Signed-off-by: Peter Krempa --- src/conf/domain_conf.c

[PATCH 09/12] virSecurityDeviceLabelDefParseXML: Use automatic memory clearing for temp strings

2021-11-22 Thread Peter Krempa
Apart from code simplification the refactor of 'model' fixes an unlikely memory leak of the string if a duplicate model is found. While the coversion of 'label' variable may seem unnecessary it will come in handy in the next patch. Signed-off-by: Peter Krempa --- src/conf/domain_conf.c | 14 +++

[PATCH 01/12] util: seclabel: Define autoptr cleanup func for virSecurityLabelDef and virSecurityDeviceLabelDef

2021-11-22 Thread Peter Krempa
Signed-off-by: Peter Krempa --- src/util/virseclabel.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/util/virseclabel.h b/src/util/virseclabel.h index 18447051fc..77bf6da2c3 100644 --- a/src/util/virseclabel.h +++ b/src/util/virseclabel.h @@ -63,4 +63,7 @@ virSecurityDeviceLabelDefCo

[PATCH 12/12] util: xml: Remove virXMLPropStringLimit and virXPathStringLimit

2021-11-22 Thread Peter Krempa
The functions have very difficult semantics where callers are not able to tell whether the property is missing or failed the length check. Only the latter produces errors. Since usage of the functions was phased out, remove them completely to avoid further broken code. Signed-off-by: Peter Krempa

[PATCH 04/12] virSecurityLabelDefParseXML: Don't reuse temporary string 'p'

2021-11-22 Thread Peter Krempa
Use separate variables for 'model' and 'relabel' properties. Signed-off-by: Peter Krempa --- src/conf/domain_conf.c | 20 src/util/virseclabel.h | 1 + 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 24de

[PATCH 00/12] Remove virXMLPropStringLimit and virXPathStringLimit

2021-11-22 Thread Peter Krempa
The functions have API which is impossible to be use correctly by callers. Refactor callers and remove the functions. Note that this patchset preserves semantics of the callers (except for removing duplicate or ignored errors). Some of the callers look fishy, but that is not addressed here. Peter

[PATCH 02/12] virSecurityLabelDef: Declare 'type' as 'virDomainSeclabelType'

2021-11-22 Thread Peter Krempa
Use the appropriate enum type instead of an int and fix the XML parser and one missing fully populated switch. Signed-off-by: Peter Krempa --- src/conf/domain_conf.c | 14 +- src/security/security_selinux.c | 3 ++- src/util/virseclabel.h | 2 +- 3 files changed,

[PATCH 08/12] virSecurityLabelDefParseXML: Don't use 'virXPathStringLimit'

2021-11-22 Thread Peter Krempa
virXPathStringLimit doesn't give callers a way to differentiate between the queried XPath being empty and the length limit being exceeded. This means that callers are either overwriting the error message or ignoring it altogether. Move the length checks into the caller. Signed-off-by: Peter Krem

[PATCH 11/12] virSecurityLabelDefParseXML: Don't use virXMLPropStringLimit

2021-11-22 Thread Peter Krempa
The function produces an error which is ignored in this code path. Signed-off-by: Peter Krempa --- src/conf/domain_conf.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d6eefed398..17ba810467 100644 --- a/src/conf/do

[PATCH 05/12] virSecurityLabelDefParseXML: Use automatic freeing for 'seclabel'

2021-11-22 Thread Peter Krempa
Signed-off-by: Peter Krempa --- src/conf/domain_conf.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index df0d033d0b..99bee98df8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7802,7 +7802,7 @@ vir

[PATCH 03/12] virSecurityLabelDefParseXML: Directly assign strings into appropriate variables

2021-11-22 Thread Peter Krempa
'seclabel->label', 'seclabel->imagelabel' and 'seclabel->baselabel' are populated by stealing the pointer from the 'p' temporary string. Remove the extra step. Signed-off-by: Peter Krempa --- src/conf/domain_conf.c | 20 1 file changed, 8 insertions(+), 12 deletions(-) diff

Re: [PATCH v6 0/4] Dirty Ring support (Libvirt)

2021-11-22 Thread Hyman
在 2021/11/22 22:48, Peter Krempa 写道: On Sat, Nov 20, 2021 at 03:20:44 -0500, huang...@chinatelecom.cn wrote: From: "Hyman Huang(黄勇)" Ping for this series. I still keep thinking the dirty ring feature is something good to have for libvirt. qemu-6.1 has supported dirty ring feature and foll

Re: [PATCH v6 4/4] qemu: support dirty ring feature

2021-11-22 Thread Hyman
在 2021/11/22 16:55, Peter Krempa 写道: On Sat, Nov 20, 2021 at 03:20:48 -0500, huang...@chinatelecom.cn wrote: From: Hyman Huang(黄勇) dirty ring feature was introduced in qemu-6.1, this patch add corresponding feature named 'dirty-ring', which enable dirty ring feature when starting vm. to en

Re: [PATCH v6 3/4] conf: introduce dirty_ring_size field

2021-11-22 Thread Hyman
在 2021/11/22 16:54, Peter Krempa 写道: On Sat, Nov 20, 2021 at 03:20:47 -0500, huang...@chinatelecom.cn wrote: From: Hyman Huang(黄勇) introduce dirty_ring_size in struct "_virDomainDef" to hold the ring size configured by user, and pass dirty_ring_size when building qemu commandline if dirty r

Re: [PATCH v6 3/4] conf: introduce dirty_ring_size field

2021-11-22 Thread Hyman
在 2021/11/22 16:50, Peter Krempa 写道: On Sat, Nov 20, 2021 at 03:20:47 -0500, huang...@chinatelecom.cn wrote: From: Hyman Huang(黄勇) introduce dirty_ring_size in struct "_virDomainDef" to hold the ring size configured by user, and pass dirty_ring_size when building qemu commandline if dirty r

Re: [PATCH 7/7] util: Make client-side polkit work even with polkit disabled

2021-11-22 Thread Ján Tomko
On a Sunday in 2021, Martin Kletzander wrote: The reason for this is twofold: - the polkit build option is documented for UNIX socket access checks - there is no server-side change or dbus call done when enabling this as it only starts a polkit agent on the client-side (actually only in virsh)

Re: [PATCH 4/7] virsh: Do not try connecting first time without polkit agent

2021-11-22 Thread Ján Tomko
On a Sunday in 2021, Martin Kletzander wrote: Trying to connect once without a polkit agent will generate an error on the server side which seems too rough given it only serves the purpose of the client (virsh in this case) to figure out that an agent is needed. Thankfully we can just try runnin

Re: [PATCH 6/7] util: Check for pkttyagent availability properly

2021-11-22 Thread Ján Tomko
On a Sunday in 2021, Martin Kletzander wrote: It does not need a tty to work, it opens its controlling terminal for user interaction and with this patch even crazy things like this work: echo 'list --name' | virsh -q >/dev/null Signed-off-by: Martin Kletzander --- src/util/virpolkit.c | 4 ++-

Re: [PATCH 5/7] util: Report errors in all code paths in virPolkitAgentCreate

2021-11-22 Thread Ján Tomko
On a Sunday in 2021, Martin Kletzander wrote: Signed-off-by: Martin Kletzander --- src/util/virpolkit.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature

Re: [PATCH 3/7] util: Add virPolkitAgentAvailable

2021-11-22 Thread Ján Tomko
On a Sunday in 2021, Martin Kletzander wrote: With this function we can decide whether to try running the polkit text agent only if it is available, removing a potential needless error saying that the agent binary does not exist, which is useful especially when running the agent before knowing wh

Re: [PATCH 2/7] util: Tiny reword fix in comment

2021-11-22 Thread Ján Tomko
On a Sunday in 2021, Martin Kletzander wrote: Automatic "Ptr " -> " *" also wreaked havoc in comments. Fix it and while at it reword the sentence so it is clear that the object is newly allocated. Signed-off-by: Martin Kletzander --- src/util/virpolkit.c | 2 +- 1 file changed, 1 insertion(+),

Re: [PATCH 1/7] virsh: Remove needless variable

2021-11-22 Thread Ján Tomko
On a Sunday in 2021, Martin Kletzander wrote: It only redundantly reflects whether pkagent != NULL. Signed-off-by: Martin Kletzander --- tools/virsh.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature

Re: [PATCH v6 0/4] Dirty Ring support (Libvirt)

2021-11-22 Thread Peter Krempa
On Sat, Nov 20, 2021 at 03:20:44 -0500, huang...@chinatelecom.cn wrote: > From: "Hyman Huang(黄勇)" > > Ping for this series. > > I still keep thinking the dirty ring feature is something good to > have for libvirt. > > qemu-6.1 has supported dirty ring feature and followed up with the > commit 0

Re: [libvirt PATCH 03/11] qemu_snapshot: revert: drop unused loadvm code

2021-11-22 Thread Pavel Hrdina
On Tue, Nov 16, 2021 at 03:30:02PM +0100, Peter Krempa wrote: > On Mon, Nov 15, 2021 at 17:22:46 +0100, Pavel Hrdina wrote: > > Now that we always restart QEMU process the loadvm code is unused and > > can be dropped. > > > > Signed-off-by: Pavel Hrdina > > --- > > src/qemu/qemu_snapshot.c | 96

Re: [libvirt PATCH 08/13] ch_cgroup: methods for cgroup mgmt in ch driver

2021-11-22 Thread Michal Prívozník
On 11/16/21 00:05, Praveen K Paladugu wrote: > > > I started refactoring this commit to have shared methods between qemu > and ch drviers for cgroup management. While doing so, I realized, I > still need a mechanism to identify what the underlying driver is : ch/qemu. > > An example of it is the

Re: [PATCH v2 3/4] conf: Introduce TCG domain features

2021-11-22 Thread Michal Prívozník
On 11/22/21 10:30, Peter Krempa wrote: > On Fri, Nov 05, 2021 at 10:35:19 +0100, Michal Privoznik wrote: >> It may come handy to be able to tweak TCG options, in this >> specific case the size of translation block cache size (tb-size). >> Since we can expect more knobs to tweak let's put them under

Re: [libvirt PATCH 02/11] qemu_snapshot: revert: always restart QEMU process for running VM

2021-11-22 Thread Peter Krempa
On Mon, Nov 22, 2021 at 12:32:03 +0100, Pavel Hrdina wrote: > On Tue, Nov 16, 2021 at 03:17:56PM +0100, Peter Krempa wrote: > > On Mon, Nov 15, 2021 at 17:22:45 +0100, Pavel Hrdina wrote: > > > Our compatibility check code isn't complete and there are cases where it > > > fails to detect incompatib

Re: [libvirt PATCH] util: fix various ATTRIBUTE_NONNULL calls

2021-11-22 Thread Ján Tomko
On a Friday in 2021, Pavel Hrdina wrote: Git bisect took me to commit where incorrect usage of ATTRIBUTE_NONNULL was introduced and caused coverity scan to fail. This patch fixes the issue where the index starts from 1 and not 0 and two other different cases. Signed-off-by: Pavel Hrdina --- src

Re: [libvirt PATCH 02/11] qemu_snapshot: revert: always restart QEMU process for running VM

2021-11-22 Thread Pavel Hrdina
On Tue, Nov 16, 2021 at 03:17:56PM +0100, Peter Krempa wrote: > On Mon, Nov 15, 2021 at 17:22:45 +0100, Pavel Hrdina wrote: > > Our compatibility check code isn't complete and there are cases where it > > fails to detect incompatible configuration and the revert fails. In > > addition future suppor

Re: [PATCH] meson: improve CPU affinity routines check

2021-11-22 Thread Roman Bogorodskiy
Martin Kletzander wrote: > On Sun, Nov 21, 2021 at 07:58:55PM +0400, Roman Bogorodskiy wrote: > >Recently, FreeBSD has got sched_get/setaffinity(3) implementations and > >the sched.h header as well [1]. To make these routines visible, > >users have to define _WITH_CPU_SET_T. > > > >This breaks c

Re: [PATCH 7/7] util: Make client-side polkit work even with polkit disabled

2021-11-22 Thread Daniel P . Berrangé
On Mon, Nov 22, 2021 at 11:03:53AM +0100, Martin Kletzander wrote: > On Mon, Nov 22, 2021 at 09:23:01AM +, Daniel P. Berrangé wrote: > > On Sun, Nov 21, 2021 at 12:10:08AM +0100, Martin Kletzander wrote: > > > The reason for this is twofold: > > > > > > - the polkit build option is documented

Re: [PATCH 7/7] util: Make client-side polkit work even with polkit disabled

2021-11-22 Thread Martin Kletzander
On Mon, Nov 22, 2021 at 09:23:01AM +, Daniel P. Berrangé wrote: On Sun, Nov 21, 2021 at 12:10:08AM +0100, Martin Kletzander wrote: The reason for this is twofold: - the polkit build option is documented for UNIX socket access checks - there is no server-side change or dbus call done when e

Re: [PATCH v2 2/2] Use virProcessGetStat

2021-11-22 Thread Daniel P . Berrangé
On Mon, Nov 22, 2021 at 10:34:38AM +0100, Martin Kletzander wrote: > On Mon, Nov 22, 2021 at 09:18:41AM +, Daniel P. Berrangé wrote: > > On Sun, Nov 21, 2021 at 12:04:26AM +0100, Martin Kletzander wrote: > > > This eliminates one incorrect parsing implementation. > > > > Please explain what wa

Re: [PATCH v2 4/4] qemu: Generate command line for tb-cache feature

2021-11-22 Thread Peter Krempa
On Fri, Nov 05, 2021 at 10:35:20 +0100, Michal Privoznik wrote: > Alright, here's the deal: to enable tb-cache one has to use > '-accel tcg,tb-size=' which then conflicts with '-machine > accel=tcg'. But sure, we can use the old -accel in this specific > case. The above is all wrong after the refa

Re: [PATCH v2 2/2] Use virProcessGetStat

2021-11-22 Thread Martin Kletzander
On Mon, Nov 22, 2021 at 09:18:41AM +, Daniel P. Berrangé wrote: On Sun, Nov 21, 2021 at 12:04:26AM +0100, Martin Kletzander wrote: This eliminates one incorrect parsing implementation. Please explain what was being done wrongly / what was the effect of the bug ? One of the implementatio

Re: [PATCH v2 3/4] conf: Introduce TCG domain features

2021-11-22 Thread Peter Krempa
On Fri, Nov 05, 2021 at 10:35:19 +0100, Michal Privoznik wrote: > It may come handy to be able to tweak TCG options, in this > specific case the size of translation block cache size (tb-size). > Since we can expect more knobs to tweak let's put them under > common element, like this: > > >

Re: [PATCH 7/7] util: Make client-side polkit work even with polkit disabled

2021-11-22 Thread Daniel P . Berrangé
On Sun, Nov 21, 2021 at 12:10:08AM +0100, Martin Kletzander wrote: > The reason for this is twofold: > > - the polkit build option is documented for UNIX socket access checks > > - there is no server-side change or dbus call done when enabling this as it > only > starts a polkit agent on the c

Re: [PATCH v2 2/2] Use virProcessGetStat

2021-11-22 Thread Daniel P . Berrangé
On Sun, Nov 21, 2021 at 12:04:26AM +0100, Martin Kletzander wrote: > This eliminates one incorrect parsing implementation. Please explain what was being done wrongly / what was the effect of the bug ? > > Signed-off-by: Martin Kletzander > --- > src/qemu/qemu_driver.c | 33 ++--

Re: [PATCH v6 2/4] qemu_command: switch accelerator option to new style

2021-11-22 Thread Hyman Huang
在 2021/11/22 16:43, Peter Krempa 写道: On Sat, Nov 20, 2021 at 03:20:46 -0500, huang...@chinatelecom.cn wrote: From: Hyman Huang(黄勇) QEMU greater than 2.9.0 support '-accel' option, change the way of assembling commandline from "accel=kvm" to "-accel kvm" when specifying accelerator. Signed-

Re: [PATCH v2 2/4] qemu: Switch to -accel

2021-11-22 Thread Peter Krempa
On Fri, Nov 05, 2021 at 10:35:18 +0100, Michal Privoznik wrote: > We currently use -machine accel=XXX which is just a syntax sugar > for -accel XXX. The former doesn't allow specifying arguments for > accelerator, because all arguments passed to -machine are > treated as arguments of machine itself

Re: [PATCH v2 1/4] qemu_command: Don't validate accelerator when building cmd line

2021-11-22 Thread Peter Krempa
On Fri, Nov 05, 2021 at 10:35:17 +0100, Michal Privoznik wrote: > The domain accelerator was validated in qemuValidateDomainDef() > which calls virQEMUCapsIsVirtTypeSupported() which reports proper > error if QEMU is not capable of KVM/TCG. There is no point in > doing the validation again when bui

Re: [PATCH v6 4/4] qemu: support dirty ring feature

2021-11-22 Thread Peter Krempa
On Sat, Nov 20, 2021 at 03:20:48 -0500, huang...@chinatelecom.cn wrote: > From: Hyman Huang(黄勇) > > dirty ring feature was introduced in qemu-6.1, this patch add > corresponding feature named 'dirty-ring', which enable > dirty ring feature when starting vm. > > to enable the feature, libvirt add

Re: [PATCH v6 3/4] conf: introduce dirty_ring_size field

2021-11-22 Thread Peter Krempa
On Sat, Nov 20, 2021 at 03:20:47 -0500, huang...@chinatelecom.cn wrote: > From: Hyman Huang(黄勇) > > introduce dirty_ring_size in struct "_virDomainDef" to hold > the ring size configured by user, and pass dirty_ring_size > when building qemu commandline if dirty ring feature enabled. > > Signed-

Re: [PATCH v6 3/4] conf: introduce dirty_ring_size field

2021-11-22 Thread Peter Krempa
On Sat, Nov 20, 2021 at 03:20:47 -0500, huang...@chinatelecom.cn wrote: > From: Hyman Huang(黄勇) > > introduce dirty_ring_size in struct "_virDomainDef" to hold > the ring size configured by user, and pass dirty_ring_size > when building qemu commandline if dirty ring feature enabled. > > Signed-

Re: [PATCH v6 2/4] qemu_command: switch accelerator option to new style

2021-11-22 Thread Peter Krempa
On Sat, Nov 20, 2021 at 03:20:46 -0500, huang...@chinatelecom.cn wrote: > From: Hyman Huang(黄勇) > > QEMU greater than 2.9.0 support '-accel' option, change the way > of assembling commandline from "accel=kvm" to "-accel kvm" when > specifying accelerator. > > Signed-off-by: Hyman Huang(黄勇) > --

Re: [PATCH v6 1/4] qemu_capabilities: introduce QEMU_CAPS_ACCEL

2021-11-22 Thread Peter Krempa
On Sat, Nov 20, 2021 at 03:20:45 -0500, huang...@chinatelecom.cn wrote: > From: Hyman Huang(黄勇) > > since the "-machine" option for accelerators is legacy, "-accel" option > may be a better mechanism. following are details: > https://lore.kernel.org/qemu-devel/3aa73987-40e8-3619-0723-9f17f7385...