Re: [PATCH 2/2] virtlo(g|ck)d: Fix exec-restart
On 3/10/21 9:37 AM, Peter Krempa wrote:
Commit 94e45d1042e broke exec-restart of virtlogd and virtlockd as the
code waiting for the daemon shutdown closed the daemons before
exec-restarting.
This reminds me of an odd issue we encountered three years ago, fixed by Daniel
https://listman.redhat.com/archives/libvir-list/2018-March/msg00298.html
I tested your patches but notice locks are still lost on re-exec.
qemu.conf:
lock_manager = "lockd"
qemu-lockd.conf:
file_lockspace_dir = "/var/lib/libvirt/lockspace"
/var/lib/libvirt/lockspace is nothing special, xfs on a local disk. After
starting a VM
# ls /var/lib/libvirt/lockspace/
a89872e150e6b9e4cbd59ef2bd289bc6cd0a8fa6fbf533c41957f77a90381e9c
# lslocks | grep lockd
virtlockd 95009 POSIX WRITE 0 0 0
/var/lib/libvirt/lockspace/a89872e150e6b9e4cbd59ef2bd289bc6cd0a8fa6fbf533c41957f77a90381e9c
virtlockd 95009 POSIX 5B WRITE 0 0 0
/run/virtlockd.pid
# systemctl reload virtlockd
# ls /var/lib/libvirt/lockspace/
a89872e150e6b9e4cbd59ef2bd289bc6cd0a8fa6fbf533c41957f77a90381e9c
# lslocks | grep lockd
#
Regards,
Jim
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1912243
Fixes: 94e45d1042e
Signed-off-by: Peter Krempa
---
src/locking/lock_daemon.c | 2 +-
src/logging/log_daemon.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 04038d2668..ffde2017ac 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -336,7 +336,7 @@ virLockDaemonExecRestartHandler(virNetDaemonPtr dmn,
void *opaque G_GNUC_UNUSED)
{
execRestart = true;
-virNetDaemonQuit(dmn);
+virNetDaemonQuitExecRestart(dmn);
}
static int
diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c
index aa76dcd329..e81de50899 100644
--- a/src/logging/log_daemon.c
+++ b/src/logging/log_daemon.c
@@ -283,7 +283,7 @@ virLogDaemonExecRestartHandler(virNetDaemonPtr dmn,
void *opaque G_GNUC_UNUSED)
{
execRestart = true;
-virNetDaemonQuit(dmn);
+virNetDaemonQuitExecRestart(dmn);
}
static int
Re: [libvirt PATCH 00/18] qemu: add support for audio backend configuration
On Wed, Mar 3, 2021 at 1:19 PM Daniel P. Berrangé wrote: > > Historically we've done almost nothing with audio backend > configuration. In QEMU we merely set QEMU_AUDIO_DRV to one > of sdl, spice, none depending on . We also have > the somewhat crazy ability to let QEMU inherit the > QEMU_AUDIO_DRV env variable from libvirtd. > > Fairly recently BHyve wanted audio backend config for OSS > so introduced the element. We designed that to allow > QEMU to later extend it, and that's what this series does. > We add types for all the QEMU backends, except the > Windows only DSound which isn't relevant for libvirt. > > The QEMU driver is updated to use this element to configure > things. QEMU has many many many more env variables for > configuring audio settings, which we can now support. These > are all deprecated since 4.0.0 though, so we also add support > for the new -audiodev argument. > > Unfortunately -audiodev isn't introspectable due to limits > in QEMU fixed by: > >https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg00653.html > > The lack of introspection isn't critical though. We can > detect existance of -audiodev by querying for '-vnc audiodev=3DNNN' > argument support. We simply lack ability to determine what QEMU > audio backends are compiled in. This means we have to delegate > error reporting to QEMU itself, which is OK. > > We'll make use of the query-audiodev command at a later date > to track future improvements to QEMU audiodev backends. > > Daniel P. Berrang=C3=A9 (18): > config: cleanup some typos / baggage wrt compiler checks > conf: stronger error reporting when parsing audio related params > conf: don't force existance of audio child elements > conf: add helper to test for sound device codec support > conf: add missing iteration over audio backends > conf: refactor OSS audio backend specific options > conf: add coverage for all QEMU audio backend types > conf: add support for audio backend for the VNC server > conf: add validation of audio backend IDs > conf: rename and improve virDomainDefFindAudioForSound > qemu: support use of elements > qemu: populate element with default config > qemu: probe for -vnc audiodev property > qemu: add support for generating -audiodev arguments > conf: introduce support for common audio settings > qemu: wire up support for common audio backend settings > conf: add support for audio backend specific settings > qemu: wire up support for backend specific audio settings > > config.h | 10 +- > docs/formatdomain.rst | 322 +++- > docs/schemas/domaincommon.rng | 384 +- > src/bhyve/bhyve_command.c | 30 +- > src/conf/domain_conf.c| 693 +- > src/conf/domain_conf.h| 125 +++- > src/conf/domain_validate.c| 67 +- > src/libvirt_private.syms | 8 +- > src/qemu/qemu_capabilities.c | 4 + > src/qemu/qemu_capabilities.h | 3 + > src/qemu/qemu_command.c | 484 +++- > src/qemu/qemu_domain.c| 110 ++- > src/qemu/qemu_validate.c | 136 +++- > .../caps_4.2.0.aarch64.xml| 1 + > .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 + > .../qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 + > .../caps_4.2.0.x86_64.xml | 1 + > .../caps_5.0.0.aarch64.xml| 1 + > .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 + > .../caps_5.0.0.riscv64.xml| 1 + > .../caps_5.0.0.x86_64.xml | 1 + > .../qemucapabilitiesdata/caps_5.1.0.sparc.xml | 1 + > .../caps_5.1.0.x86_64.xml | 1 + > .../caps_5.2.0.aarch64.xml| 1 + > .../qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 + > .../caps_5.2.0.riscv64.xml| 1 + > .../qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 + > .../caps_5.2.0.x86_64.xml | 1 + > .../caps_6.0.0.x86_64.xml | 1 + > .../redefine.xml | 1 + > .../disk_snapshot_redefine.xml| 1 + > .../external_vm_redefine.xml | 1 + > .../full_domain.xml | 1 + > .../qemudomainsnapshotxml2xmlout/metadata.xml | 1 + > .../ppc64-modern-bulk-result-conf.xml | 1 + > .../ppc64-modern-bulk-result-live.xml | 1 + > .../ppc64-modern-individual-result-conf.xml | 1 + > .../ppc64-modern-individual-result-live.xml | 1 + > .../x86-modern-bulk-result-conf.xml | 1 + > .../x86-modern-bulk-result-live.xml | 1 + > .../x86-modern-individual-add-result-conf.xml | 1 + > .../x86-modern-individual-add-result-live.xml | 1 + > .../x86-old-bulk-result-conf.xml
Re: [PATCH v4 0/4] Drop deprecated floppy config & bogus -drive if=T
On 3/11/21 2:52 AM, Markus Armbruster wrote: v4: * PATCH 3: Move a declaration into a loop [Richard] * PATCH 4: Drop a superfluous call to drive_check_orphaned() [Daniel], fix comments [John] v3: * PATCH 1: New [Daniel] v2: * Rebased, straightforward conflict with commit f5d33dd51f "hw/block/fdc: Remove the check_media_rate property" resolved * PATCH 2: Commit message fixed [Kevin] Markus Armbruster (4): docs/system/deprecated: Fix note on fdc drive properties fdc: Drop deprecated floppy configuration fdc: Inline fdctrl_connect_drives() into fdctrl_realize_common() blockdev: Drop deprecated bogus -drive interface type docs/system/deprecated.rst | 33 -- docs/system/removed-features.rst | 56 +++ include/sysemu/blockdev.h| 1 - blockdev.c | 37 +- hw/block/fdc.c | 73 +--- softmmu/vl.c | 11 +- tests/qemu-iotests/172 | 31 +- tests/qemu-iotests/172.out | 562 +-- 8 files changed, 82 insertions(+), 722 deletions(-) Reviewed-by: John Snow Tentatively staged, pending CI: https://gitlab.com/jsnow/qemu/-/pipelines/269277138 --js
Re: [PATCH 05/14] migrate: remove QMP/HMP commands for speed, downtime and cache size
On 11/03/21 19:33, Daniel P. Berrangé wrote: On Thu, Mar 11, 2021 at 07:18:54PM +0100, Paolo Bonzini wrote: On 11/03/21 12:54, Dr. David Alan Gilbert wrote: * Daniel P. Berrangé (berra...@redhat.com) wrote: The generic 'migrate_set_parameters' command handle all types of param. Only the QMP commands were documented in the deprecations page, but the rationale for deprecating applies equally to HMP, and the replacements exist. Furthermore the HMP commands are just shims to the QMP commands, so removing the latter breaks the former unless they get re-implemented. Signed-off-by: Daniel P. Berrangé Yes OK; ouch that's going to break my 7 years of instinctive 'migrate_set_speed 10G' typing, but it's probably the right thing to do. migrate_set_speed should remain if it is not changed to have a sane default. Define sane ? The default is 1 Gib/s since: commit 7590a2ae091fde8bb72d5df93977ab9707e23242 Author: Laurent Vivier Date: Mon Sep 21 16:49:57 2020 +0200 migration: increase max-bandwidth to 128 MiB/s (1 Gib/s) Oh, I missed that! I was still thinking of the old 32 MiB/s value. Paolo
Re: [PATCH 6/6] lib: Drop internal virXXXPtr typedefs
On Thu, Mar 11, 2021 at 06:54:20PM +0100, Michal Privoznik wrote: > Historically, we declared pointer type to our types: > > typedef struct _virXXX virXXX; > typedef virXXX *virXXXPtr; > > But usefulness of such declaration is questionable, at best. > Unfortunately, we can't drop every such declaration - we have to > carry some over, because they are part of public API (e.g. > virDomainPtr). But for internal types - we can do drop them and > use what every other C project uses 'virXXX *'. > > This change was generated by a very ugly shell script that > generated sed script which was then called over each file in the > repository. For the shell script refer to the cover letter: > > $URL > > Signed-off-by: Michal Privoznik > --- > docs/advanced-tests.rst |2 +- > docs/api_extension.html.in|2 +- > docs/coding-style.rst |2 +- snip > tools/virt-login-shell-helper.c |4 +- > tools/vsh-table.c | 36 +- > tools/vsh-table.h | 12 +- > tools/vsh.c |4 +- > 732 files changed, 29237 insertions(+), 30131 deletions(-) Converting every single file at the same time in one commit is guaranting backporting conflict hell. eg if I'm backporting a patch from src/qemu, it is much saner if I can cherry-pick the "Ptr" conversion from src/qemu and only deal with conflicts in that, not the entire soure tree. So if we're going to do this conversion, IMHO the actual commits need to be way more granular. At the very least I think this needs to be split per driver, with tests/ associated with their driver. The util/ directory needs to be split up and likewise the conf/ directory per object type I think. Also dropping usage of the "Ptr" should be separate from dropping the typedefs themselves as I think that'll make conflicts easier to deal with. 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 :|
Re: [PATCH 05/14] migrate: remove QMP/HMP commands for speed, downtime and cache size
On Thu, Mar 11, 2021 at 07:18:54PM +0100, Paolo Bonzini wrote: > On 11/03/21 12:54, Dr. David Alan Gilbert wrote: > > * Daniel P. Berrangé (berra...@redhat.com) wrote: > > > The generic 'migrate_set_parameters' command handle all types of param. > > > > > > Only the QMP commands were documented in the deprecations page, but the > > > rationale for deprecating applies equally to HMP, and the replacements > > > exist. Furthermore the HMP commands are just shims to the QMP commands, > > > so removing the latter breaks the former unless they get re-implemented. > > > > > > Signed-off-by: Daniel P. Berrangé > > > > Yes OK; ouch that's going to break my 7 years of instinctive > > 'migrate_set_speed 10G' typing, but it's probably the right thing to do. > > migrate_set_speed should remain if it is not changed to have a sane default. Define sane ? The default is 1 Gib/s since: commit 7590a2ae091fde8bb72d5df93977ab9707e23242 Author: Laurent Vivier Date: Mon Sep 21 16:49:57 2020 +0200 migration: increase max-bandwidth to 128 MiB/s (1 Gib/s) 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 :|
Re: [PATCH 05/14] migrate: remove QMP/HMP commands for speed, downtime and cache size
On 11/03/21 12:54, Dr. David Alan Gilbert wrote: * Daniel P. Berrangé (berra...@redhat.com) wrote: The generic 'migrate_set_parameters' command handle all types of param. Only the QMP commands were documented in the deprecations page, but the rationale for deprecating applies equally to HMP, and the replacements exist. Furthermore the HMP commands are just shims to the QMP commands, so removing the latter breaks the former unless they get re-implemented. Signed-off-by: Daniel P. Berrangé Yes OK; ouch that's going to break my 7 years of instinctive 'migrate_set_speed 10G' typing, but it's probably the right thing to do. migrate_set_speed should remain if it is not changed to have a sane default. Paolo
[PATCH 4/6] virconftypes: Fix name of virCapsGuestArchPtr
The name is supposed to be virCapsGuestArchPtr not ..ptr. Signed-off-by: Michal Privoznik --- src/conf/virconftypes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h index c51241cfa5..4658d0de9c 100644 --- a/src/conf/virconftypes.h +++ b/src/conf/virconftypes.h @@ -37,7 +37,7 @@ typedef struct _virCapsGuest virCapsGuest; typedef virCapsGuest *virCapsGuestPtr; typedef struct _virCapsGuestArch virCapsGuestArch; -typedef virCapsGuestArch *virCapsGuestArchptr; +typedef virCapsGuestArch *virCapsGuestArchPtr; typedef struct _virCapsGuestDomain virCapsGuestDomain; typedef virCapsGuestDomain *virCapsGuestDomainPtr; -- 2.26.2
[PATCH 0/6] Drop internal virXXXPtr typedefs
This is patch set that implement dropping of virXXXPtr typedefs as
discussed here:
https://listman.redhat.com/archives/libvir-list/2021-March/msg00427.html
and it does is in big bang style, just like suggested in the discussion.
Patches can be found here:
https://gitlab.com/MichalPrivoznik/libvirt/-/tree/ptr_remove
And of course pipeline is green:
https://gitlab.com/MichalPrivoznik/libvirt/-/pipelines/269102351
Patch 6/6 is intentionally cut off as it has ~ 6MiB. I'll replace $URL
in its commit message with link to this cover letter, once I send it.
I've played around with sed and came to the following ugly, horrible and
ineffective bash script:
===
#!/bin/bash
SED_SCRIPT="sed_script"
export IFS=$'\n'
PRESERVED=( $(git grep -h "^typedef.*Ptr;$" -- include/) )
for i in $(git grep -h "^typedef.*Ptr;$"); do
subst=1
for p in ${PRESERVED[*]}; do
if [[ $i == *$p* ]]; then
subst=0
fi
done
if [ $subst -ne 1 ]; then
continue;
fi
PTR=$(echo $i | cut -d'*' -f2 | cut -d';' -f1);
if [[ $i == *struct* ]]; then
SRC=$(echo ${i#"typedef "} | cut -d' ' -f1-2);
else
SRC=$(echo ${i#"typedef "} | cut -d' ' -f1) ;
fi
if [ "$1" -eq 0 ]; then
echo "s/${PTR} /${SRC} */g";
elif [ "$1" -eq 1 ]; then
echo "s/${PTR}/${SRC} */g";
echo "/^typedef.*\*${SRC} \*;$/D";
fi
done > ${SED_SCRIPT}
for i in $(git ls-files); do
if [ -f $i -a ! -L $i ]; then
echo $i;
sed -i -f ${SED_SCRIPT} $i;
fi
done
rm ${SED_SCRIPT}
===
I've ran it like following:
./script.sh 0 && ./script.sh 1
Mostly, because in the first run I wanted to fix following pattern:
virXXXPtr variable;
and in the second:
static virXXXPtr
function()
And just like always, I had to fix some stuff by hand. For instance, in
vbox there is this function virVBoxSnapshotConfHardDiskPtrByLocation()
and of course there's also virVBoxSnapshotConfHardDiskPtr.
Then, I've dropped some empty lines, mostly from patterns like this:
typedef struct _virXXX virXXX;
struct _virXXX {
Michal Prívozník (6):
virsysinfo: Define and use auto cleanup func for virSysinfoDef
properly
gendispatch: Don't use virXXXPtr for internal types
syntax-check: Fix and rename virSecurity rule
virconftypes: Fix name of virCapsGuestArchPtr
lib: Put some variable declarations on individual lines
lib: Drop internal virXXXPtr typedefs
build-aux/syntax-check.mk |4 +-
docs/advanced-tests.rst |2 +-
docs/api_extension.html.in|2 +-
docs/coding-style.rst |2 +-
docs/internals/command.html.in| 12 +-
docs/internals/rpc.html.in| 32 +-
scripts/esx_vi_generator.py |6 +-
scripts/hyperv_wmi_generator.py |6 +-
src/access/viraccessdriver.h | 52 +-
src/access/viraccessdrivernop.c | 46 +-
src/access/viraccessdriverpolkit.c| 52 +-
src/access/viraccessdriverstack.c | 82 +-
src/access/viraccessdriverstack.h |4 +-
src/access/viraccessmanager.c | 78 +-
src/access/viraccessmanager.h | 57 +-
src/admin/admin_remote.c | 56 +-
src/admin/admin_server.c | 34 +-
src/admin/admin_server.h | 26 +-
src/admin/admin_server_dispatch.c | 129 +-
src/admin/admin_server_dispatch.h |8 +-
src/admin/libvirt-admin.c |8 +-
src/bhyve/bhyve_capabilities.c| 26 +-
src/bhyve/bhyve_capabilities.h|8 +-
src/bhyve/bhyve_command.c | 121 +-
src/bhyve/bhyve_command.h | 14 +-
src/bhyve/bhyve_conf.c| 18 +-
src/bhyve/bhyve_conf.h|9 +-
src/bhyve/bhyve_device.c | 30 +-
src/bhyve/bhyve_device.h |6 +-
src/bhyve/bhyve_domain.c | 36 +-
src/bhyve/bhyve_domain.h |7 +-
src/bhyve/bhyve_driver.c | 161 +-
src/bhyve/bhyve_driver.h |6 +-
src/bhyve/bhyve_monitor.c | 34 +-
src/bhyve/bhyve_monitor.h |7 +-
src/bhyve/bhyve_parse_command.c | 40 +-
src/bhyve/bhyve_parse_command.h |4 +-
src/bhyve/bhyve_process.c | 52 +-
src/bhyve/bhyve_process.h | 16 +-
src/bhyve/bhyve_utils.h | 25 +-
src/conf/backup_conf.c| 54 +-
src/conf/backup_conf.h| 24 +-
src/conf/capabilities.c | 240 +-
src/conf/capabilities.h | 116 +-
[PATCH 1/6] virsysinfo: Define and use auto cleanup func for virSysinfoDef properly
What we are using really is heap allocated structure rather than
stack allocated. And for that it's better to use g_autoptr() +
G_DEFINE_AUTOPTR_CLEANUP_FUNC() combo, as Glib documentation for
g_auto() reads:
This is meant to be used with stack-allocated structures and
non-pointer types. For the (more commonly used) pointer
version, see g_autoptr().
This will be even more visible, when virSysinfoDefPtr type is
gone. Stay tuned.
Fixes: cee3a900a0d6a8fc79554db22dc262632fe487a6
Signed-off-by: Michal Privoznik
---
src/util/virsysinfo.c | 8
src/util/virsysinfo.h | 2 +-
tests/sysinfotest.c | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c
index 0016028254..1c8193cc58 100644
--- a/src/util/virsysinfo.c
+++ b/src/util/virsysinfo.c
@@ -314,7 +314,7 @@ virSysinfoParsePPCProcessor(const char *base,
virSysinfoDefPtr ret)
virSysinfoDefPtr
virSysinfoReadPPC(void)
{
-g_auto(virSysinfoDefPtr) ret = NULL;
+g_autoptr(virSysinfoDef) ret = NULL;
g_autofree char *outbuf = NULL;
ret = g_new0(virSysinfoDef, 1);
@@ -436,7 +436,7 @@ virSysinfoParseARMProcessor(const char *base,
virSysinfoDefPtr ret)
virSysinfoDefPtr
virSysinfoReadARM(void)
{
-g_auto(virSysinfoDefPtr) ret = NULL;
+g_autoptr(virSysinfoDef) ret = NULL;
g_autofree char *outbuf = NULL;
/* Some ARM systems have DMI tables available. */
@@ -602,7 +602,7 @@ virSysinfoParseS390Processor(const char *base,
virSysinfoDefPtr ret)
virSysinfoDefPtr
virSysinfoReadS390(void)
{
-g_auto(virSysinfoDefPtr) ret = NULL;
+g_autoptr(virSysinfoDef) ret = NULL;
g_autofree char *outbuf = NULL;
ret = g_new0(virSysinfoDef, 1);
@@ -1212,7 +1212,7 @@ virSysinfoParseX86Memory(const char *base,
virSysinfoDefPtr ret)
virSysinfoDefPtr
virSysinfoReadDMI(void)
{
-g_auto(virSysinfoDefPtr) ret = NULL;
+g_autoptr(virSysinfoDef) ret = NULL;
g_autofree char *outbuf = NULL;
g_autoptr(virCommand) cmd = NULL;
diff --git a/src/util/virsysinfo.h b/src/util/virsysinfo.h
index 6b25969a4b..6ae4ba7bf3 100644
--- a/src/util/virsysinfo.h
+++ b/src/util/virsysinfo.h
@@ -157,7 +157,7 @@ void virSysinfoChassisDefFree(virSysinfoChassisDefPtr def);
void virSysinfoOEMStringsDefFree(virSysinfoOEMStringsDefPtr def);
void virSysinfoDefFree(virSysinfoDefPtr def);
-G_DEFINE_AUTO_CLEANUP_FREE_FUNC(virSysinfoDefPtr, virSysinfoDefFree, NULL);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virSysinfoDef, virSysinfoDefFree);
int virSysinfoFormat(virBufferPtr buf, virSysinfoDefPtr def)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
diff --git a/tests/sysinfotest.c b/tests/sysinfotest.c
index 3b418955d0..e40a4564a7 100644
--- a/tests/sysinfotest.c
+++ b/tests/sysinfotest.c
@@ -91,7 +91,7 @@ testSysinfo(const void *data)
{
const struct testSysinfoData *testdata = data;
const char *sysfsActualData;
-g_auto(virSysinfoDefPtr) ret = NULL;
+g_autoptr(virSysinfoDef) ret = NULL;
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
g_autofree char *sysinfo = NULL;
g_autofree char *cpuinfo = NULL;
--
2.26.2
[PATCH 6/6] lib: Drop internal virXXXPtr typedefs
Historically, we declared pointer type to our types: typedef struct _virXXX virXXX; typedef virXXX *virXXXPtr; But usefulness of such declaration is questionable, at best. Unfortunately, we can't drop every such declaration - we have to carry some over, because they are part of public API (e.g. virDomainPtr). But for internal types - we can do drop them and use what every other C project uses 'virXXX *'. This change was generated by a very ugly shell script that generated sed script which was then called over each file in the repository. For the shell script refer to the cover letter: $URL Signed-off-by: Michal Privoznik --- docs/advanced-tests.rst |2 +- docs/api_extension.html.in|2 +- docs/coding-style.rst |2 +- docs/internals/command.html.in| 12 +- docs/internals/rpc.html.in| 32 +- scripts/esx_vi_generator.py |6 +- scripts/hyperv_wmi_generator.py |6 +- src/access/viraccessdriver.h | 52 +- src/access/viraccessdrivernop.c | 46 +- src/access/viraccessdriverpolkit.c| 52 +- src/access/viraccessdriverstack.c | 82 +- src/access/viraccessdriverstack.h |4 +- src/access/viraccessmanager.c | 78 +- src/access/viraccessmanager.h | 57 +- src/admin/admin_remote.c | 56 +- src/admin/admin_server.c | 34 +- src/admin/admin_server.h | 26 +- src/admin/admin_server_dispatch.c | 129 +- src/admin/admin_server_dispatch.h |8 +- src/admin/libvirt-admin.c |8 +- src/bhyve/bhyve_capabilities.c| 26 +- src/bhyve/bhyve_capabilities.h|8 +- src/bhyve/bhyve_command.c | 124 +- src/bhyve/bhyve_command.h | 14 +- src/bhyve/bhyve_conf.c| 18 +- src/bhyve/bhyve_conf.h|9 +- src/bhyve/bhyve_device.c | 30 +- src/bhyve/bhyve_device.h |6 +- src/bhyve/bhyve_domain.c | 36 +- src/bhyve/bhyve_domain.h |7 +- src/bhyve/bhyve_driver.c | 162 +- src/bhyve/bhyve_driver.h |6 +- src/bhyve/bhyve_monitor.c | 34 +- src/bhyve/bhyve_monitor.h |7 +- src/bhyve/bhyve_parse_command.c | 40 +- src/bhyve/bhyve_parse_command.h |4 +- src/bhyve/bhyve_process.c | 52 +- src/bhyve/bhyve_process.h | 16 +- src/bhyve/bhyve_utils.h | 25 +- src/conf/backup_conf.c| 54 +- src/conf/backup_conf.h| 24 +- src/conf/capabilities.c | 240 +- src/conf/capabilities.h | 116 +- src/conf/checkpoint_conf.c| 80 +- src/conf/checkpoint_conf.h| 23 +- src/conf/cpu_conf.c | 78 +- src/conf/cpu_conf.h | 71 +- src/conf/device_conf.c| 36 +- src/conf/device_conf.h| 42 +- src/conf/domain_addr.c| 316 +-- src/conf/domain_addr.h| 128 +- src/conf/domain_audit.c | 96 +- src/conf/domain_audit.h | 76 +- src/conf/domain_capabilities.c| 70 +- src/conf/domain_capabilities.h| 39 +- src/conf/domain_conf.c| 2509 - src/conf/domain_conf.h| 967 --- src/conf/domain_event.c | 552 ++-- src/conf/domain_event.h | 178 +- src/conf/domain_nwfilter.c| 18 +- src/conf/domain_nwfilter.h|6 +- src/conf/domain_validate.c| 64 +- src/conf/domain_validate.h| 10 +- src/conf/interface_conf.c | 76 +- src/conf/interface_conf.h | 18 +- src/conf/moment_conf.c|8 +- src/conf/moment_conf.h|8 +- src/conf/netdev_bandwidth_conf.c | 12 +- src/conf/netdev_bandwidth_conf.h |6 +- src/conf/netdev_vlan_conf.c |4 +- src/conf/netdev_vlan_conf.h |4 +- src/conf/netdev_vport_profile_conf.c |6 +- src/conf/netdev_vport_profile_conf.h |4 +- src/conf/network_conf.c | 198 +- src/conf/network_conf.h | 105 +- src/conf/network_event.c | 26 +-
[PATCH 5/6] lib: Put some variable declarations on individual lines
In short, virXXXPtr type is going away. With bing bang. And to
help us rewrite the code with a sed script, it's better if each
variable is declared on its own line.
Signed-off-by: Michal Privoznik
---
src/bhyve/bhyve_command.c | 5 -
src/bhyve/bhyve_driver.c | 3 ++-
src/conf/domain_addr.c| 3 ++-
src/conf/domain_conf.c| 3 ++-
src/conf/numa_conf.c | 3 ++-
src/conf/nwfilter_params.c| 3 ++-
src/conf/virnetworkobj.c | 3 ++-
src/hypervisor/domain_driver.c| 3 ++-
src/hypervisor/virhostdev.c | 3 ++-
src/libxl/libxl_driver.c | 9 ++---
src/libxl/xen_common.c| 12
src/libxl/xen_xl.c| 24 +++
src/libxl/xen_xm.c| 3 ++-
src/lxc/lxc_controller.c | 3 ++-
src/lxc/lxc_driver.c | 12
src/network/bridge_driver.c | 9 +++--
src/nwfilter/nwfilter_ebiptables_driver.c | 6 --
src/openvz/openvz_driver.c| 3 ++-
src/qemu/qemu_agent.c | 3 ++-
src/qemu/qemu_domain.c| 3 ++-
src/qemu/qemu_driver.c| 15 +-
src/qemu/qemu_monitor_json.c | 14 +
src/rpc/virnetserverclient.c | 3 ++-
src/security/security_stack.c | 3 ++-
src/test/test_driver.c| 3 ++-
src/util/virconf.c| 11 ---
src/util/virnetdevbandwidth.c | 3 ++-
src/util/virnetdevbandwidth.h | 3 ++-
src/vz/vz_driver.c| 3 ++-
src/vz/vz_sdk.c | 9 ++---
tests/virnetdaemontest.c | 12
tests/virpcitest.c| 9 ++---
32 files changed, 139 insertions(+), 65 deletions(-)
diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
index c4c64788b9..e3c2921dea 100644
--- a/src/bhyve/bhyve_command.c
+++ b/src/bhyve/bhyve_command.c
@@ -917,7 +917,10 @@ virBhyveProcessBuildGrubbhyveCmd(virDomainDefPtr def,
const char *devmap_file,
char **devicesmap_out)
{
-virDomainDiskDefPtr hdd, cd, userdef, diskdef;
+virDomainDiskDefPtr hdd;
+virDomainDiskDefPtr cd;
+virDomainDiskDefPtr userdef;
+virDomainDiskDefPtr diskdef;
virCommandPtr cmd;
unsigned int best_idx = UINT_MAX;
size_t i;
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 8363259d4c..158fee4445 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -681,7 +681,8 @@ bhyveConnectDomainXMLToNative(virConnectPtr conn,
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
bhyveConnPtr privconn = conn->privateData;
virDomainDefPtr def = NULL;
-virCommandPtr cmd = NULL, loadcmd = NULL;
+virCommandPtr cmd = NULL;
+virCommandPtr loadcmd = NULL;
char *ret = NULL;
virCheckFlags(0, NULL);
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index fa08f5381d..8b927689f8 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -2082,7 +2082,8 @@ int
virDomainUSBAddressSetAddHub(virDomainUSBAddressSetPtr addrs,
virDomainHubDefPtr hub)
{
-virDomainUSBAddressHubPtr targetHub = NULL, newHub = NULL;
+virDomainUSBAddressHubPtr targetHub = NULL;
+virDomainUSBAddressHubPtr newHub = NULL;
int ret = -1;
int targetPort;
g_autofree char *portStr = NULL;
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 08e09362ee..47756ff0be 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17621,7 +17621,8 @@ virDomainChrDefPtr
virDomainChrRemove(virDomainDefPtr vmdef,
virDomainChrDefPtr chr)
{
-virDomainChrDefPtr ret = NULL, **arrPtr = NULL;
+virDomainChrDefPtr ret = NULL;
+virDomainChrDefPtr **arrPtr = NULL;
size_t i, *cntPtr = NULL;
if (virDomainChrGetDomainPtrsInternal(vmdef, chr->deviceType,
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 338d779e90..64b93fd7d1 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -780,7 +780,8 @@ virDomainNumaDefNodeDistanceParseXML(virDomainNumaPtr def,
}
for (i = 0; i < sibling; i++) {
-virDomainNumaDistancePtr ldist, rdist;
+virDomainNumaDistancePtr ldist;
+virDomainNumaDistancePtr rdist;
unsigned int sibling_id, sibling_value;
/* siblings are in order of parsing or explicitly numbered */
diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c
index 1be492759a..5fa5d4fedb 100644
--- a/src/conf/nwfilter_params.c
+++ b/src/conf/nwfilter_params.c
@@ -402,7 +402,8 @@
[PATCH 3/6] syntax-check: Fix and rename virSecurity rule
The aim of virSecurity rule is to discourage from using plain
virSecurityManager*() APIs within QEMU driver in favor of their
qemuSecurity*() counterparts. The reason is simple: namespaces;
virSecurityManager*() needs additional
virSecurityManagerTransactionCommit() call to enter given
namespace and do its work from there. And that's exactly what
those qemuSecurity*() wrappers do.
To help us ensure correctness (from this POV), we have a
syntax-check rule that forbids any occurrence of
"virSecurityManager" string under src/qemu/ (except for
qemu_security of course).
But with if we want to remove virSecurityManagerPtr type, then we
have to allow "virSecurityManager *". Therefore, change the rule
so that no call of a function with "virSecurityManager" prefix is
allowed. And also change the name to better reflect what is going
on.
Signed-off-by: Michal Privoznik
---
build-aux/syntax-check.mk | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index 2f4f932a5b..c71b608e66 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -1049,10 +1049,10 @@ sc_prohibit_sysconf_pagesize:
halt='use virGetSystemPageSize[KB] instead of sysconf(_SC_PAGESIZE)' \
$(_sc_search_regexp)
-sc_prohibit_virSecurity:
+sc_prohibit_virSecurityManager:
@$(VC_LIST_EXCEPT) | $(GREP) 'src/qemu/' | \
$(GREP) -v 'src/qemu/qemu_security' | \
- xargs $(GREP) -Pn 'virSecurityManager(?!Ptr)' /dev/null && \
+ xargs $(GREP) -Pn 'virSecurityManager\S*\(' /dev/null && \
{ echo '$(ME): prefer qemuSecurity wrappers' 1>&2; exit 1; } ||
:
sc_prohibit_pthread_create:
--
2.26.2
[PATCH 2/6] gendispatch: Don't use virXXXPtr for internal types
The use of virXXXPtr is going away soon, therefore use 'virXXX *'
instead.
Signed-off-by: Michal Privoznik
---
src/rpc/gendispatch.pl | 66 +-
1 file changed, 33 insertions(+), 33 deletions(-)
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index 0020273d9e..9dcd8c3e89 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -113,7 +113,7 @@ sub name_to_TypeName {
sub get_conn_type {
if ($structprefix eq "admin") {
-return "virNetDaemonPtr";
+return "virNetDaemon *";
} else {
return "virConnectPtr";
}
@@ -499,10 +499,10 @@ elsif ($mode eq "server") {
# First we print out a function declaration for the
# real dispatcher body
print "static int ${name}(\n";
-print "virNetServerPtr server,\n";
-print "virNetServerClientPtr client,\n";
-print "virNetMessagePtr msg,\n";
-print "virNetMessageErrorPtr rerr";
+print "virNetServer *server,\n";
+print "virNetServerClient *client,\n";
+print "virNetMessage *msg,\n";
+print "struct virNetMessageError *rerr";
if ($argtype ne "void") {
print ",\n$argtype *args";
}
@@ -516,10 +516,10 @@ elsif ($mode eq "server") {
# fixed function signature, for use in the dispatcher
# table. This simply callers the real dispatcher method
print "static int ${name}Helper(\n";
-print "virNetServerPtr server,\n";
-print "virNetServerClientPtr client,\n";
-print "virNetMessagePtr msg,\n";
-print "virNetMessageErrorPtr rerr,\n";
+print "virNetServer *server,\n";
+print "virNetServerClient *client,\n";
+print "virNetMessage *msg,\n";
+print "struct virNetMessageError *rerr,\n";
print "void *args$argann,\n";
print "void *ret$retann)\n";
print "{\n";
@@ -641,7 +641,7 @@ elsif ($mode eq "server") {
}
push(@args_list, "$1");
push(@args_list, "n$1");
-push(@getters_list, "if
(virTypedParamsDeserialize((virTypedParameterRemotePtr) args->$1.$1_val,\n" .
+push(@getters_list, "if
(virTypedParamsDeserialize((struct _virTypedParameterRemote *)
args->$1.$1_val,\n" .
"
args->$1.$1_len,\n" .
"
$2,\n" .
"
&$1,\n" .
@@ -687,7 +687,7 @@ elsif ($mode eq "server") {
} elsif ($args_member =~ m/^admin_nonnull_(server) (\S+);/) {
my $type_name = name_to_TypeName($1);
-push(@vars_list, "virNet${type_name}Ptr $2 = NULL");
+push(@vars_list, "virNet${type_name} *$2 = NULL");
push(@getters_list,
"if (!($2 = get_nonnull_$1($conn_var,
args->$2)))\n" .
"goto cleanup;\n");
@@ -697,8 +697,8 @@ elsif ($mode eq "server") {
} elsif ($args_member =~ m/^admin_nonnull_(client) (\S+);/) {
my $type_name = name_to_TypeName($1);
-push(@vars_list, "virNetServerPtr srv = NULL");
-push(@vars_list, "virNetServer${type_name}Ptr $2 = NULL");
+push(@vars_list, "virNetServer *srv = NULL");
+push(@vars_list, "virNetServer${type_name} *$2 = NULL");
push(@getters_list,
"if (!(srv = get_nonnull_server($conn_var,
args->$2.srv)))\n" .
"goto cleanup;\n");
@@ -930,11 +930,11 @@ elsif ($mode eq "server") {
my $type_name = name_to_TypeName($1);
if ($1 eq "client") {
-push(@vars_list, "virNetServer${type_name}Ptr $2 =
NULL");
+push(@vars_list, "virNetServer${type_name} *$2 =
NULL");
push(@ret_list, "make_nonnull_$1(>$2, $2);\n");
push(@ret_list, "make_nonnull_server(>$2.srv,
srv);\n");
} else {
-push(@vars_list, "virNet${type_name}Ptr $2 = NULL");
+push(@vars_list, "virNet${type_name} *$2 = NULL");
push(@ret_list, "make_nonnull_$1(>$2, $2);");
}
@@ -955,13 +955,13 @@ elsif ($mode eq "server") {
push(@ret_list, "if (virTypedParamsSerialize($1,
$1_len,\n" .
"$2,\n" .
-"
(virTypedParameterRemotePtr *) >$1.$1_val,\n" .
+
Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
On 11/03/21 15:08, Markus Armbruster wrote:
I would rather keep the OptsVisitor here. Do the same check for JSON
syntax that you have in qobject_input_visitor_new_str, and whenever
you need to walk all -object arguments, use something like this:
typedef struct ObjectArgument {
const char *id;
QDict *json;/* or NULL for QemuOpts */
QSIMPLEQ_ENTRY(ObjectArgument) next;
}
I already had patches in my queue to store -object in a GSList of
dictionaries, changing it to use the above is easy enough.
I think I'd prefer following -display's precedence. See my reply to
Kevin for details.
Yeah, I got independently to the same conclusion and posted patches for
that. I was scared that visit_type_ObjectOptions was too much for
OptsVisitor but it seems to work...
Paolo
[PATCH v2] xenParseVif: Refactor parser
Use g_strsplit to split the string and avoid use of stack'd strings.
Signed-off-by: Peter Krempa
---
src/libxl/xen_common.c | 130 +
1 file changed, 39 insertions(+), 91 deletions(-)
v2:
- helper variables are made const and strings are borrowed from the
array returned by g_strsplit instead of copying/freeing
- second argument of xenParseVifBridge turned const to silence compiler
diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c
index c56815d7fc..0d3a3f994a 100644
--- a/src/libxl/xen_common.c
+++ b/src/libxl/xen_common.c
@@ -1027,7 +1027,7 @@ xenParseCharDev(virConfPtr conf, virDomainDefPtr def,
const char *nativeFormat)
static int
-xenParseVifBridge(virDomainNetDefPtr net, char *bridge)
+xenParseVifBridge(virDomainNetDefPtr net, const char *bridge)
{
char *vlanstr;
unsigned int tag;
@@ -1141,104 +1141,53 @@ xenParseVif(char *entry, const char *vif_typename)
{
virDomainNetDefPtr net = NULL;
virDomainNetDefPtr ret = NULL;
-char *script = NULL;
-char model[10];
-char type[10];
-char ip[128];
-char mac[18];
-char bridge[50];
-char vifname[50];
-char rate[50];
-char *key;
-
-bridge[0] = '\0';
-mac[0] = '\0';
-ip[0] = '\0';
-model[0] = '\0';
-type[0] = '\0';
-vifname[0] = '\0';
-rate[0] = '\0';
-
-key = entry;
-while (key) {
-char *data;
-char *nextkey = strchr(key, ',');
-
-if (!(data = strchr(key, '=')))
+g_auto(GStrv) keyvals = NULL;
+GStrv keyval;
+const char *script = NULL;
+const char *model = NULL;
+const char *type = NULL;
+const char *ip = NULL;
+const char *mac = NULL;
+const char *bridge = NULL;
+const char *vifname = NULL;
+const char *rate = NULL;
+
+keyvals = g_strsplit(entry, ",", 0);
+
+for (keyval = keyvals; keyval && *keyval; keyval++) {
+const char *key = *keyval;
+char *val = strchr(key, '=');
+
+virSkipSpaces();
+
+if (!val)
return NULL;
-data++;
+
+val++;
if (STRPREFIX(key, "mac=")) {
-int len = nextkey ? (nextkey - data) : strlen(data);
-if (virStrncpy(mac, data, len, sizeof(mac)) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("MAC address %s too big for destination"),
- data);
-return NULL;
-}
+mac = val;
} else if (STRPREFIX(key, "bridge=")) {
-int len = nextkey ? (nextkey - data) : strlen(data);
-if (virStrncpy(bridge, data, len, sizeof(bridge)) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Bridge %s too big for destination"),
- data);
-return NULL;
-}
+bridge = val;
} else if (STRPREFIX(key, "script=")) {
-int len = nextkey ? (nextkey - data) : strlen(data);
-VIR_FREE(script);
-script = g_strndup(data, len);
+script = val;
} else if (STRPREFIX(key, "model=")) {
-int len = nextkey ? (nextkey - data) : strlen(data);
-if (virStrncpy(model, data, len, sizeof(model)) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Model %s too big for destination"),
- data);
-return NULL;
-}
+model = val;
} else if (STRPREFIX(key, "type=")) {
-int len = nextkey ? (nextkey - data) : strlen(data);
-if (virStrncpy(type, data, len, sizeof(type)) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Type %s too big for destination"),
- data);
-return NULL;
-}
+type = val;
} else if (STRPREFIX(key, "vifname=")) {
-int len = nextkey ? (nextkey - data) : strlen(data);
-if (virStrncpy(vifname, data, len, sizeof(vifname)) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Vifname %s too big for destination"),
- data);
-return NULL;
-}
+vifname = val;
} else if (STRPREFIX(key, "ip=")) {
-int len = nextkey ? (nextkey - data) : strlen(data);
-if (virStrncpy(ip, data, len, sizeof(ip)) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("IP %s too big for destination"), data);
-return NULL;
-}
+ip = val;
} else if (STRPREFIX(key, "rate=")) {
-int len = nextkey ? (nextkey - data) : strlen(data);
-if (virStrncpy(rate, data, len, sizeof(rate)) < 0) {
-
Re: [PATCH] Add 'interleave' to the sub-element for video device in rng file
On a Thursday in 2021, Kristina Hanicova wrote: Previously, validation of XML failed if sub-elements of video device were in different order. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1825769 Signed-off-by: Kristina Hanicova --- docs/schemas/domaincommon.rng | 216 +- 1 file changed, 109 insertions(+), 107 deletions(-) Reviewed-by: Ján Tomko and pushed Jano signature.asc Description: PGP signature
Re: [libvirt PATCH] spec: Drop BuildDepends on make
On a Thursday in 2021, Andrea Bolognani wrote: make is only used for the syntax-check tests, which we are explicitly skipping when building RPMs. Signed-off-by: Andrea Bolognani --- libvirt.spec.in | 1 - mingw-libvirt.spec.in | 1 - 2 files changed, 2 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 0/2] Properly fix backup job termination
On a Thursday in 2021, Peter Krempa wrote: Peter Krempa (2): backup: Store 'apiFlags' in private section of virDomainBackupDef qemuBackupJobTerminate: Fix job termination for inactive VMs src/conf/backup_conf.h | 2 ++ src/qemu/qemu_backup.c | 33 ++--- src/qemu/qemu_process.c | 9 ++--- 3 files changed, 26 insertions(+), 18 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
[PATCH 0/2] Properly fix backup job termination
Peter Krempa (2): backup: Store 'apiFlags' in private section of virDomainBackupDef qemuBackupJobTerminate: Fix job termination for inactive VMs src/conf/backup_conf.h | 2 ++ src/qemu/qemu_backup.c | 33 ++--- src/qemu/qemu_process.c | 9 ++--- 3 files changed, 26 insertions(+), 18 deletions(-) -- 2.29.2
[PATCH 2/2] qemuBackupJobTerminate: Fix job termination for inactive VMs
Commit cb29e4e801d didn't take into account that the VM can be inactive
when it's destroyed. This means that the job would remain active also
when the VM became inactive.
To fix this properly:
1) Remove the bogus VM liveness check and early return
(reverts the aforementioned commit)
2) Conditionalize the stats assignment only when the stats object is
present
(properly fix the crash when VM dies when reconnecting)
3) end the asyncjob only when it was already set
(prevent corruption of priv->jobs_queued)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1937598
Fixes: cb29e4e801d
Signed-off-by: Peter Krempa
---
I didn't come up with a reasonable split so that I'd avoid the
'technically' 3 changes in one patch so that it'd still make sense.
src/qemu/qemu_backup.c | 29 +++--
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index f6096f643f..f91d632715 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -583,27 +583,28 @@ qemuBackupJobTerminate(virDomainObjPtr vm,
}
}
-if (!virDomainObjIsActive(vm))
-return;
-
-qemuDomainJobInfoUpdateTime(priv->job.current);
+if (priv->job.current) {
+qemuDomainJobInfoUpdateTime(priv->job.current);
-g_clear_pointer(>job.completed, qemuDomainJobInfoFree);
-priv->job.completed = qemuDomainJobInfoCopy(priv->job.current);
+g_clear_pointer(>job.completed, qemuDomainJobInfoFree);
+priv->job.completed = qemuDomainJobInfoCopy(priv->job.current);
-priv->job.completed->stats.backup.total = priv->backup->push_total;
-priv->job.completed->stats.backup.transferred =
priv->backup->push_transferred;
-priv->job.completed->stats.backup.tmp_used = priv->backup->pull_tmp_used;
-priv->job.completed->stats.backup.tmp_total = priv->backup->pull_tmp_total;
+priv->job.completed->stats.backup.total = priv->backup->push_total;
+priv->job.completed->stats.backup.transferred =
priv->backup->push_transferred;
+priv->job.completed->stats.backup.tmp_used =
priv->backup->pull_tmp_used;
+priv->job.completed->stats.backup.tmp_total =
priv->backup->pull_tmp_total;
-priv->job.completed->status = jobstatus;
-priv->job.completed->errmsg = g_strdup(priv->backup->errmsg);
+priv->job.completed->status = jobstatus;
+priv->job.completed->errmsg = g_strdup(priv->backup->errmsg);
-qemuDomainEventEmitJobCompleted(priv->driver, vm);
+qemuDomainEventEmitJobCompleted(priv->driver, vm);
+}
virDomainBackupDefFree(priv->backup);
priv->backup = NULL;
-qemuDomainObjEndAsyncJob(priv->driver, vm);
+
+if (priv->job.asyncJob == QEMU_ASYNC_JOB_BACKUP)
+qemuDomainObjEndAsyncJob(priv->driver, vm);
}
--
2.29.2
[PATCH 1/2] backup: Store 'apiFlags' in private section of virDomainBackupDef
'qemuBackupJobTerminate' needs the API flags to see whether
VIR_DOMAIN_BACKUP_BEGIN_REUSE_EXTERNAL. Unfortunately when called via
qemuProcessReconnect()->qemuProcessStop() early (e.g. if the qemu
process died while we were reconnecting) the job is cleared temporarily
so that other APIs can be called. This would mean that we couldn't clean
up the files in some cases.
Save the 'apiFlags' inside the backup object and set it from the
'qemuDomainJobObj' 'apiFlags' member when reconnecting to a VM.
Signed-off-by: Peter Krempa
---
src/conf/backup_conf.h | 2 ++
src/qemu/qemu_backup.c | 4 +++-
src/qemu/qemu_process.c | 9 ++---
3 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/src/conf/backup_conf.h b/src/conf/backup_conf.h
index bda2bdcfe4..2902f39fb7 100644
--- a/src/conf/backup_conf.h
+++ b/src/conf/backup_conf.h
@@ -99,6 +99,8 @@ struct _virDomainBackupDef {
unsigned long long pull_tmp_total;
char *errmsg; /* error message of failed sub-blockjob */
+
+unsigned int apiFlags; /* original flags used when starting the job */
};
typedef enum {
diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index 6ce29c28e1..f6096f643f 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -560,7 +560,7 @@ qemuBackupJobTerminate(virDomainObjPtr vm,
qemuDomainObjPrivatePtr priv = vm->privateData;
size_t i;
-if (!(priv->job.apiFlags & VIR_DOMAIN_BACKUP_BEGIN_REUSE_EXTERNAL) &&
+if (!(priv->backup->apiFlags & VIR_DOMAIN_BACKUP_BEGIN_REUSE_EXTERNAL) &&
(priv->backup->type == VIR_DOMAIN_BACKUP_TYPE_PULL ||
(priv->backup->type == VIR_DOMAIN_BACKUP_TYPE_PUSH &&
jobstatus != QEMU_DOMAIN_JOB_STATUS_COMPLETED))) {
@@ -766,6 +766,8 @@ qemuBackupBegin(virDomainObjPtr vm,
if (def->type == VIR_DOMAIN_BACKUP_TYPE_PULL)
pull = true;
+def->apiFlags = flags;
+
/* we'll treat this kind of backup job as an asyncjob as it uses some of
the
* infrastructure for async jobs. We'll allow standard modify-type jobs
* as the interlocking of conflicting operations is handled on the block
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 89ede27751..971a270793 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -96,6 +96,7 @@
#include "virthreadjob.h"
#include "virutil.h"
#include "storage_source.h"
+#include "backup_conf.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -8315,12 +8316,14 @@ qemuProcessReconnect(void *opaque)
g_clear_object(>identity);
VIR_FREE(data);
+cfg = virQEMUDriverGetConfig(driver);
+priv = obj->privateData;
+
qemuDomainObjRestoreJob(obj, );
if (oldjob.asyncJob == QEMU_ASYNC_JOB_MIGRATION_IN)
stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED;
-
-cfg = virQEMUDriverGetConfig(driver);
-priv = obj->privateData;
+if (oldjob.asyncJob == QEMU_ASYNC_JOB_BACKUP && priv->backup)
+priv->backup->apiFlags = oldjob.apiFlags;
/* expect that libvirt might have crashed during VM start, so prevent
* cleanup of transient disks */
--
2.29.2
Re: [PATCH 3/3] selinux: Remove 'make' dependency
On Thu, 2021-03-11 at 14:44 +, Daniel P. Berrangé wrote: > On Thu, Mar 11, 2021 at 03:32:23PM +0100, Andrea Bolognani wrote: > > Mh, fair enough. I guess we can drop > > > > BuildRequires: make > > > > from our .spec files then... I'll post a patch right away. > > Our RPMs build with a .git present, so they'll run syntax-checks even > downstream. We explicitly disable syntax-check: https://gitlab.com/libvirt/libvirt/-/blob/f11f32326f163dda28143e9495d9bbc5d4869f6d/libvirt.spec.in#L1301-1304 The MinGW spec doesn't have a %check section at all, for obvious reasons. Patch here: https://listman.redhat.com/archives/libvir-list/2021-March/msg00526.html -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 3/3] selinux: Remove 'make' dependency
On Thu, Mar 11, 2021 at 03:32:23PM +0100, Andrea Bolognani wrote: > On Thu, 2021-03-11 at 13:26 +, Daniel P. Berrangé wrote: > > On Thu, Mar 11, 2021 at 02:00:55PM +0100, Andrea Bolognani wrote: > > > On Thu, 2021-03-11 at 12:33 +, Daniel P. Berrangé wrote: > > > > Sure it would be nice if there was a meson extension that dealt > > > > with SELinux, but we need to implement something that works with > > > > the meson releases that exist today. If meson gains selinux > > > > support in future may be we can consider it then. > > > > > > We still need make for syntax-check though, so rewriting the SELinux > > > bits to Meson doesn't allow us to drop the dependency. And, > > > considering how complex and widely used the syntax-check logic is, I > > > don't see that being reimplemented anytime soon. > > > > That's only part of the test suite, and we don't even include > > syntax-check if not running from git, because it is only targetted > > at upstream maintainers. So that's quite different from including > > use of make in the primary build process. That is just not ok IMHO. > > Mh, fair enough. I guess we can drop > > BuildRequires: make > > from our .spec files then... I'll post a patch right away. Our RPMs build with a .git present, so they'll run syntax-checks even downstream. 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 :|
[libvirt PATCH] spec: Drop BuildDepends on make
make is only used for the syntax-check tests, which we are
explicitly skipping when building RPMs.
Signed-off-by: Andrea Bolognani
---
libvirt.spec.in | 1 -
mingw-libvirt.spec.in | 1 -
2 files changed, 2 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 8d8b900fbb..f9af330186 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -266,7 +266,6 @@ BuildRequires: python3-docutils
BuildRequires: gcc
BuildRequires: meson >= 0.54.0
BuildRequires: ninja-build
-BuildRequires: make
BuildRequires: git
%if 0%{?fedora} || 0%{?rhel} > 7
BuildRequires: perl-interpreter
diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in
index 0686cbaf78..288f533d52 100644
--- a/mingw-libvirt.spec.in
+++ b/mingw-libvirt.spec.in
@@ -54,7 +54,6 @@ BuildRequires: libxslt
BuildRequires: python3
BuildRequires: perl-interpreter
BuildRequires: perl(Getopt::Long)
-BuildRequires: make
BuildRequires: meson
BuildRequires: ninja-build
BuildRequires: python3-docutils
--
2.26.2
Re: [PATCH 3/3] selinux: Remove 'make' dependency
On Thu, 2021-03-11 at 13:26 +, Daniel P. Berrangé wrote: > On Thu, Mar 11, 2021 at 02:00:55PM +0100, Andrea Bolognani wrote: > > On Thu, 2021-03-11 at 12:33 +, Daniel P. Berrangé wrote: > > > Sure it would be nice if there was a meson extension that dealt > > > with SELinux, but we need to implement something that works with > > > the meson releases that exist today. If meson gains selinux > > > support in future may be we can consider it then. > > > > We still need make for syntax-check though, so rewriting the SELinux > > bits to Meson doesn't allow us to drop the dependency. And, > > considering how complex and widely used the syntax-check logic is, I > > don't see that being reimplemented anytime soon. > > That's only part of the test suite, and we don't even include > syntax-check if not running from git, because it is only targetted > at upstream maintainers. So that's quite different from including > use of make in the primary build process. That is just not ok IMHO. Mh, fair enough. I guess we can drop BuildRequires: make from our .spec files then... I'll post a patch right away. -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
Paolo Bonzini writes:
> On 11/03/21 11:38, Markus Armbruster wrote:
>> Here's a differently terrible hack. We have
>> keyval_parse() visitor
>> optarg > QObject > QAPI type
>> Idea: hack the QObject. If we're working for -object, and QObject
>> maps
>> key "qom-type" to value "memory-backend-ram", get the value of
>> host-nodes, and if it's a string, parse it into a list like the opts
>> visitor does, and put that back, replacing the string value.
>> Same for other uses of Memdev and NumaNodeOptions with -object, if
>> they
>> exist.
>
> This doesn't help with backwards compatibility, since keyval loses the
> first of host-nodes=0,host-nodes=4.
You're right, I missed the fact that we rely on both hacks working
together for the full syntax.
> I would rather keep the OptsVisitor here. Do the same check for JSON
> syntax that you have in qobject_input_visitor_new_str, and whenever
> you need to walk all -object arguments, use something like this:
>
> typedef struct ObjectArgument {
> const char *id;
> QDict *json;/* or NULL for QemuOpts */
> QSIMPLEQ_ENTRY(ObjectArgument) next;
> }
>
> I already had patches in my queue to store -object in a GSList of
> dictionaries, changing it to use the above is easy enough.
I think I'd prefer following -display's precedence. See my reply to
Kevin for details.
Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
Kevin Wolf writes:
> Am 11.03.2021 um 12:24 hat Peter Krempa geschrieben:
>> On Thu, Mar 11, 2021 at 09:37:11 +0100, Kevin Wolf wrote:
>> > Am 11.03.2021 um 08:47 hat Peter Krempa geschrieben:
>> > > On Wed, Mar 10, 2021 at 18:30:44 +0100, Kevin Wolf wrote:
>> > > > Am 10.03.2021 um 15:31 hat Paolo Bonzini geschrieben:
>> > > > > On 10/03/21 15:22, Peter Krempa wrote:
>>
>> [...]
>>
>> > > -object
>> > > memory-backend-ram,id=ram-node2,size=24578621440,host-nodes=1-2,host-nodes=5,host-nodes=7,policy=bind
>> >
>> > Oh, we have ranges, too... That makes the compatibility code even
>> > nastier to write. I doubt that we can implement this in the keyval
>> > parser alone without touching the visitor. :-/
>> >
>> > > If any of the above is to be deprecated we'll need to adjust our
>> > > JSON->commandline generator accordignly.
>> > >
>> > > Luckily the 'host-nodes' is storeable as a bitmap and the generator is
>> > > actually modular to allow plugging an array interpretor which actually
>> > > does the above conversion from a JSON array.
>> > >
>> > > So, what is the preferred syntax here? Additionally we might need a
>> > > witness property to detect when to use the new syntax if basing it on
>> > > the object-add qapification will not be enough.
>> >
>> > The list syntax supported by the keyval parser is the one you know from
>> > -blockdev: host-nodes.0=3,host-nodes.1=7, ...
>>
>> I think that should be easy enough to convert to.
>
> We could also support JSON syntax in QEMU. That would probably be even
> more convenient for libvirt?
Cleanly QAPIfied options like -blockdev do
if (optarg[0] == '{') {
parse @optarg as JSON with qobject_from_json()
convert to C type with qobject_input_visitor_new()
} else {
parse @optarg with keyval_parse()
convert to C type with qobject_input_visitor_new_keyval()
}
Options where compatibility problems defeat keyval_parse() can do
if (optarg[0] == '{') {
parse @optarg as JSON with qobject_from_json()
convert to C type with qobject_input_visitor_new()
} else {
parse the old way
convert to C type somehow
}
Precedence: -display. There, the old way is an ad hoc parser, and the
conversion to C type DisplayOptions is manual. For -object, the old way
would be QemuOpts, and the conversion would use the opts visitor.
>> Is it safe to do right away (with the old parser?). Otherwise we need
>> to agree on something which will let us detect when it's safe to
>> change.
>
> Neither keyval nor JSON syntax work with the old QemuOpts parser.
>
> What is the usual way to do this for command line options? If we don't
> have a good way there, we can always tie it to something in the QAPI
> schema. If we still get this solved in time for 6.0, we could use the
> existence of ObjectOptions. Or all else failing, we can use a feature
> flag.
You should not look for types in output of query-qmp-schema, only for
commands and events. To discourage looking for types, query-qmp-schema
masks the names of user-defined types.
A feature flag is fine as last resort. That's what they were designed
for.
Re: [PATCH 3/3] selinux: Remove 'make' dependency
On Thu, Mar 11, 2021 at 02:00:55PM +0100, Andrea Bolognani wrote: > On Thu, 2021-03-11 at 12:33 +, Daniel P. Berrangé wrote: > > On Thu, Mar 11, 2021 at 07:29:03AM -0500, Neal Gompa wrote: > > > On Thu, Mar 11, 2021 at 6:35 AM Daniel P. Berrangé > > > wrote: > > > > On Wed, Mar 10, 2021 at 01:50:43PM -0500, Neal Gompa wrote: > > > > > On Wed, Mar 10, 2021 at 7:43 AM Nikola Knazekova > > > > > wrote: > > > > > > Compile the policy using a shell script executed by meson. > > > > > > > > > > This smells like a bad idea, because we're not relying on the > > > > > framework that SELinux policies are supposed to be built with. I don't > > > > > think we should do this. > > > > > > > > The important part is the use of tools for compiling the policy. The way > > > > you glue them into a build system is a app specific, and it makes no > > > > sense > > > > to use SELinux provided Makefiles, when our build system is meson. > > > > > > Let's say I buy that argument (I don't). Even with that argument, this > > > patch is wrong because it makes assumptions about how SELinux policies > > > are structured on-disk. For example, the install directory is wrong, > > > since it should be share/selinux/packages, not > > > share/selinux/packages/targeted. If I were to accept that I might be > > > wrong about the directory in the previous statement, that means that > > > we're still wrong, because we don't have builds for mls and minimal > > > policy targets. Finally, we're missing the policy interface file. > > > > Well if there are bugs like these, that's what this review is intended > > to catch, and they'll need to be addressed before this can merge. > > > > > This sounds like you need to work with Meson and selinux-policy > > > upstream to add support for natively building policy modules with > > > Meson itself. > > > > Sure it would be nice if there was a meson extension that dealt > > with SELinux, but we need to implement something that works with > > the meson releases that exist today. If meson gains selinux > > support in future may be we can consider it then. > > We still need make for syntax-check though, so rewriting the SELinux > bits to Meson doesn't allow us to drop the dependency. And, > considering how complex and widely used the syntax-check logic is, I > don't see that being reimplemented anytime soon. That's only part of the test suite, and we don't even include syntax-check if not running from git, because it is only targetted at upstream maintainers. So that's quite different from including use of make in the primary build process. That is just not ok IMHO. 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 :|
[PATCH] Add 'interleave' to the sub-element for video device in rng file
Previously, validation of XML failed if sub-elements of video device were in different order. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1825769 Signed-off-by: Kristina Hanicova --- docs/schemas/domaincommon.rng | 216 +- 1 file changed, 109 insertions(+), 107 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index f7f804adc0..e6db2f5b74 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4009,120 +4009,122 @@ --> - - - - - - - - -qemu -vhostuser - - - - - - -io -on -off - - - - - - - - - - -vga -cirrus -vmvga -xen -vbox -virtio -gop -none -bochs -ramfb - - - + + + + + + + + + + qemu + vhostuser + + + + + + + io + on + off + + + + + + + + -qxl + + vga + cirrus + vmvga + xen + vbox + virtio + gop + none + bochs + ramfb + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + qxl - - - - - - + + + + + + + + + + + + + + + + + + + - + + + - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
Re: [PATCH 3/3] selinux: Remove 'make' dependency
On Thu, 2021-03-11 at 12:33 +, Daniel P. Berrangé wrote: > On Thu, Mar 11, 2021 at 07:29:03AM -0500, Neal Gompa wrote: > > On Thu, Mar 11, 2021 at 6:35 AM Daniel P. Berrangé > > wrote: > > > On Wed, Mar 10, 2021 at 01:50:43PM -0500, Neal Gompa wrote: > > > > On Wed, Mar 10, 2021 at 7:43 AM Nikola Knazekova > > > > wrote: > > > > > Compile the policy using a shell script executed by meson. > > > > > > > > This smells like a bad idea, because we're not relying on the > > > > framework that SELinux policies are supposed to be built with. I don't > > > > think we should do this. > > > > > > The important part is the use of tools for compiling the policy. The way > > > you glue them into a build system is a app specific, and it makes no sense > > > to use SELinux provided Makefiles, when our build system is meson. > > > > Let's say I buy that argument (I don't). Even with that argument, this > > patch is wrong because it makes assumptions about how SELinux policies > > are structured on-disk. For example, the install directory is wrong, > > since it should be share/selinux/packages, not > > share/selinux/packages/targeted. If I were to accept that I might be > > wrong about the directory in the previous statement, that means that > > we're still wrong, because we don't have builds for mls and minimal > > policy targets. Finally, we're missing the policy interface file. > > Well if there are bugs like these, that's what this review is intended > to catch, and they'll need to be addressed before this can merge. > > > This sounds like you need to work with Meson and selinux-policy > > upstream to add support for natively building policy modules with > > Meson itself. > > Sure it would be nice if there was a meson extension that dealt > with SELinux, but we need to implement something that works with > the meson releases that exist today. If meson gains selinux > support in future may be we can consider it then. We still need make for syntax-check though, so rewriting the SELinux bits to Meson doesn't allow us to drop the dependency. And, considering how complex and widely used the syntax-check logic is, I don't see that being reimplemented anytime soon. So perhaps we could file a bug against the SELinux package asking for native Meson support, hope that it will be implemented by the time we get around to rewrite syntax-check, and just use make in the meantime. -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH v2 0/3] ui: add support for 'secret' object to provide VNC/SPICE passwords
On Thu, Mar 11, 2021 at 11:43:40AM +, Daniel P. Berrangé wrote: > This fixes a long standing limitation of the VNC/SPICE code which was > unable to securely accept passswords on the CLI, instead requiring use > of separate monitor commands after startup. > > In v2: > > - Dropped patch removing ACL commands, as it will move to a bigger >deprecation cleanup series > - Rebased and resolved conflicts Added to UI queue. thanks, Gerd
Re: [PATCH 3/3] selinux: Remove 'make' dependency
On Thu, Mar 11, 2021 at 07:29:03AM -0500, Neal Gompa wrote:
> On Thu, Mar 11, 2021 at 6:35 AM Daniel P. Berrangé
> wrote:
> >
> > On Wed, Mar 10, 2021 at 01:50:43PM -0500, Neal Gompa wrote:
> > > On Wed, Mar 10, 2021 at 7:43 AM Nikola Knazekova
> > > wrote:
> > > >
> > > > From: Vit Mojzis
> > > >
> > > > Compile the policy using a shell script executed by meson.
> > > >
> > > > Signed-off-by: Vit Mojzis
> > > > ---
> > > > libvirt.spec.in | 12
> > > > meson.build | 12
> > > > selinux/compile_policy.sh | 39 +++
> > > > selinux/meson.build | 23 +++
> > > > 4 files changed, 74 insertions(+), 12 deletions(-)
> > > > create mode 100755 selinux/compile_policy.sh
> > > > create mode 100644 selinux/meson.build
> > > >
> > > > diff --git a/libvirt.spec.in b/libvirt.spec.in
> > > > index db08d91043..de664084fa 100644
> > > > --- a/libvirt.spec.in
> > > > +++ b/libvirt.spec.in
> > > > @@ -1240,14 +1240,6 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y'
> > > > %{_specdir}/%{name}.spec)
> > > > %{?arg_login_shell}
> > > >
> > > > %meson_build
> > > > -%if 0%{?with_selinux}
> > > > -# SELinux policy (originally from selinux-policy-contrib)
> > > > -# this policy module will override the production module
> > > > -cd selinux
> > > > -
> > > > -make -f %{_datadir}/selinux/devel/Makefile %{modulename}.pp
> > > > -bzip2 -9 %{modulename}.pp
> > > > -%endif
> > > >
> > > > %install
> > > > rm -fr %{buildroot}
> > > > @@ -1332,10 +1324,6 @@ mv
> > > > $RPM_BUILD_ROOT%{_datadir}/systemtap/tapset/libvirt_qemu_probes.stp \
> > > > %endif
> > > > %endif
> > > >
> > > > -%if 0%{?with_selinux}
> > > > -install -D -m 0644 selinux/%{modulename}.pp.bz2
> > > > %{buildroot}%{_datadir}/selinux/packages/%{selinuxtype}/%{modulename}.pp.bz2
> > > > -%endif
> > > > -
> > > > %check
> > > > # Building on slow archs, like emulated s390x in Fedora copr, requires
> > > > # raising the test timeout
> > > > diff --git a/meson.build b/meson.build
> > > > index c81c6ab205..d060e441b5 100644
> > > > --- a/meson.build
> > > > +++ b/meson.build
> > > > @@ -2183,6 +2183,18 @@ endif
> > > >
> > > > subdir('build-aux')
> > > >
> > > > +os_release = run_command('grep', '^ID=', '/etc/os-release').stdout()
> > > > +os_version = run_command('grep', '^VERSION_ID=',
> > > > '/etc/os-release').stdout().split('=')
> > > > +if (os_version.length() == 2)
> > > > + os_version = os_version[1]
> > > > +else
> > > > + os_version = 0
> > > > +endif
> > > > +
> > > > +if ((os_release.contains('fedora') and
> > > > os_version.version_compare('>32')) or
> > > > +(os_release.contains('rhel') and os_version.version_compare('>7')))
> > > > + subdir('selinux')
> > > > +endif
> > > >
> > > > # install pkgconfig files
> > > > pkgconfig_files = [
> > > > diff --git a/selinux/compile_policy.sh b/selinux/compile_policy.sh
> > > > new file mode 100755
> > > > index 00..02780e4aed
> > > > --- /dev/null
> > > > +++ b/selinux/compile_policy.sh
> > > > @@ -0,0 +1,39 @@
> > > > +#!/bin/sh
> > > > +set -x
> > > > +
> > > > +if [[ $# -ne 5 ]] ; then
> > > > +echo "Usage: compile_policy.sh .te .if .fc
> > > > .pp "
> > > > +exit 1
> > > > +fi
> > > > +
> > > > +# checkmodule requires consistent file names
> > > > +MODULE_NAME=$(basename -- "$1")
> > > > +MODULE_NAME=${MODULE_NAME%.*}
> > > > +
> > > > +M4PARAM="-D enable_mcs -D distro_redhat -D hide_broken_symptoms -D
> > > > mls_num_sens=16 -D mls_num_cats=1024 -D mcs_num_cats=1024"
> > > > +SHAREDIR="/usr/share/selinux"
> > > > +HEADERDIR="$SHAREDIR/devel/include"
> > > > +M4SUPPORT=$(echo $HEADERDIR/support/*.spt)
> > > > +HEADER_LAYERS=$(find "/usr/share/selinux/devel/include"/* -maxdepth 0
> > > > -type d | grep -v "/usr/share/selinux/devel/include/support")
> > > > +HEADER_INTERFACES=""
> > > > +for LAYER in $HEADER_LAYERS
> > > > +do
> > > > +HEADER_INTERFACES="$HEADER_INTERFACES $(echo $LAYER/*.if)"
> > > > +done
> > > > +
> > > > +# prepare temp folder
> > > > +mkdir -p $5
> > > > +# remove old trash from the temp folder
> > > > +rm -rf "$5/iferror.m4 $5/all_interfaces.conf $5/$MODULE_NAME.*"
> > > > +# tmp/all_interfaces.conf
> > > > +echo "ifdef(\`__if_error',\`m4exit(1)')" > $5/iferror.m4
> > > > +echo "divert(-1)" > $5/all_interfaces.conf
> > > > +m4 $M4SUPPORT $HEADER_INTERFACES $2 $5/iferror.m4 | sed -e
> > > > s/dollarsstar/\$\$\*/g >> $5/all_interfaces.conf
> > > > +echo "divert" >> $5/all_interfaces.conf
> > > > +# tmp/%.mod
> > > > +m4 $M4PARAM -s $M4SUPPORT $5/all_interfaces.conf $1 >
> > > > $5/$MODULE_NAME.tmp
> > > > +/usr/bin/checkmodule -M -m $5/$MODULE_NAME.tmp -o $5/$MODULE_NAME.mod
> > > > +# tmp/%.mod.fc
> > > > +m4 $M4PARAM $M4SUPPORT $3 > $5/$MODULE_NAME.mod.fc
> > > > +# %.pp
> > > > +/usr/bin/semodule_package -o $4 -m $5/$MODULE_NAME.mod -f
> > > > $5/$MODULE_NAME.mod.fc
> >
> > Can you change this to use Python, since
Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
On Thu, Mar 11, 2021 at 12:41:42 +0100, Kevin Wolf wrote: > Am 11.03.2021 um 12:24 hat Peter Krempa geschrieben: > > On Thu, Mar 11, 2021 at 09:37:11 +0100, Kevin Wolf wrote: > > > Am 11.03.2021 um 08:47 hat Peter Krempa geschrieben: > > > > On Wed, Mar 10, 2021 at 18:30:44 +0100, Kevin Wolf wrote: > > > > > Am 10.03.2021 um 15:31 hat Paolo Bonzini geschrieben: > > > > > > On 10/03/21 15:22, Peter Krempa wrote: > > > > [...] > > > > > > -object > > > > memory-backend-ram,id=ram-node2,size=24578621440,host-nodes=1-2,host-nodes=5,host-nodes=7,policy=bind > > > > > > Oh, we have ranges, too... That makes the compatibility code even > > > nastier to write. I doubt that we can implement this in the keyval > > > parser alone without touching the visitor. :-/ > > > > > > > If any of the above is to be deprecated we'll need to adjust our > > > > JSON->commandline generator accordignly. > > > > > > > > Luckily the 'host-nodes' is storeable as a bitmap and the generator is > > > > actually modular to allow plugging an array interpretor which actually > > > > does the above conversion from a JSON array. > > > > > > > > So, what is the preferred syntax here? Additionally we might need a > > > > witness property to detect when to use the new syntax if basing it on > > > > the object-add qapification will not be enough. > > > > > > The list syntax supported by the keyval parser is the one you know from > > > -blockdev: host-nodes.0=3,host-nodes.1=7, ... > > > > I think that should be easy enough to convert to. > > We could also support JSON syntax in QEMU. That would probably be even > more convenient for libvirt? Definitely yes. We already do have the JSON internal representation, so outputing JSON directly just skips the commandlinificator. > > Is it safe to do right away (with the old parser?). Otherwise we need > > to agree on something which will let us detect when it's safe to > > change. > > Neither keyval nor JSON syntax work with the old QemuOpts parser. > > What is the usual way to do this for command line options? If we don't > have a good way there, we can always tie it to something in the QAPI > schema. If we still get this solved in time for 6.0, we could use the > existence of ObjectOptions. Or all else failing, we can use a feature > flag. Yup, in this release I'd use what I have for detecting qapification of -object. If we can do JSON with this, it would be ideal.
Re: [PATCH 3/3] selinux: Remove 'make' dependency
On Thu, Mar 11, 2021 at 6:35 AM Daniel P. Berrangé wrote:
>
> On Wed, Mar 10, 2021 at 01:50:43PM -0500, Neal Gompa wrote:
> > On Wed, Mar 10, 2021 at 7:43 AM Nikola Knazekova
> > wrote:
> > >
> > > From: Vit Mojzis
> > >
> > > Compile the policy using a shell script executed by meson.
> > >
> > > Signed-off-by: Vit Mojzis
> > > ---
> > > libvirt.spec.in | 12
> > > meson.build | 12
> > > selinux/compile_policy.sh | 39 +++
> > > selinux/meson.build | 23 +++
> > > 4 files changed, 74 insertions(+), 12 deletions(-)
> > > create mode 100755 selinux/compile_policy.sh
> > > create mode 100644 selinux/meson.build
> > >
> > > diff --git a/libvirt.spec.in b/libvirt.spec.in
> > > index db08d91043..de664084fa 100644
> > > --- a/libvirt.spec.in
> > > +++ b/libvirt.spec.in
> > > @@ -1240,14 +1240,6 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y'
> > > %{_specdir}/%{name}.spec)
> > > %{?arg_login_shell}
> > >
> > > %meson_build
> > > -%if 0%{?with_selinux}
> > > -# SELinux policy (originally from selinux-policy-contrib)
> > > -# this policy module will override the production module
> > > -cd selinux
> > > -
> > > -make -f %{_datadir}/selinux/devel/Makefile %{modulename}.pp
> > > -bzip2 -9 %{modulename}.pp
> > > -%endif
> > >
> > > %install
> > > rm -fr %{buildroot}
> > > @@ -1332,10 +1324,6 @@ mv
> > > $RPM_BUILD_ROOT%{_datadir}/systemtap/tapset/libvirt_qemu_probes.stp \
> > > %endif
> > > %endif
> > >
> > > -%if 0%{?with_selinux}
> > > -install -D -m 0644 selinux/%{modulename}.pp.bz2
> > > %{buildroot}%{_datadir}/selinux/packages/%{selinuxtype}/%{modulename}.pp.bz2
> > > -%endif
> > > -
> > > %check
> > > # Building on slow archs, like emulated s390x in Fedora copr, requires
> > > # raising the test timeout
> > > diff --git a/meson.build b/meson.build
> > > index c81c6ab205..d060e441b5 100644
> > > --- a/meson.build
> > > +++ b/meson.build
> > > @@ -2183,6 +2183,18 @@ endif
> > >
> > > subdir('build-aux')
> > >
> > > +os_release = run_command('grep', '^ID=', '/etc/os-release').stdout()
> > > +os_version = run_command('grep', '^VERSION_ID=',
> > > '/etc/os-release').stdout().split('=')
> > > +if (os_version.length() == 2)
> > > + os_version = os_version[1]
> > > +else
> > > + os_version = 0
> > > +endif
> > > +
> > > +if ((os_release.contains('fedora') and
> > > os_version.version_compare('>32')) or
> > > +(os_release.contains('rhel') and os_version.version_compare('>7')))
> > > + subdir('selinux')
> > > +endif
> > >
> > > # install pkgconfig files
> > > pkgconfig_files = [
> > > diff --git a/selinux/compile_policy.sh b/selinux/compile_policy.sh
> > > new file mode 100755
> > > index 00..02780e4aed
> > > --- /dev/null
> > > +++ b/selinux/compile_policy.sh
> > > @@ -0,0 +1,39 @@
> > > +#!/bin/sh
> > > +set -x
> > > +
> > > +if [[ $# -ne 5 ]] ; then
> > > +echo "Usage: compile_policy.sh .te .if .fc
> > > .pp "
> > > +exit 1
> > > +fi
> > > +
> > > +# checkmodule requires consistent file names
> > > +MODULE_NAME=$(basename -- "$1")
> > > +MODULE_NAME=${MODULE_NAME%.*}
> > > +
> > > +M4PARAM="-D enable_mcs -D distro_redhat -D hide_broken_symptoms -D
> > > mls_num_sens=16 -D mls_num_cats=1024 -D mcs_num_cats=1024"
> > > +SHAREDIR="/usr/share/selinux"
> > > +HEADERDIR="$SHAREDIR/devel/include"
> > > +M4SUPPORT=$(echo $HEADERDIR/support/*.spt)
> > > +HEADER_LAYERS=$(find "/usr/share/selinux/devel/include"/* -maxdepth 0
> > > -type d | grep -v "/usr/share/selinux/devel/include/support")
> > > +HEADER_INTERFACES=""
> > > +for LAYER in $HEADER_LAYERS
> > > +do
> > > +HEADER_INTERFACES="$HEADER_INTERFACES $(echo $LAYER/*.if)"
> > > +done
> > > +
> > > +# prepare temp folder
> > > +mkdir -p $5
> > > +# remove old trash from the temp folder
> > > +rm -rf "$5/iferror.m4 $5/all_interfaces.conf $5/$MODULE_NAME.*"
> > > +# tmp/all_interfaces.conf
> > > +echo "ifdef(\`__if_error',\`m4exit(1)')" > $5/iferror.m4
> > > +echo "divert(-1)" > $5/all_interfaces.conf
> > > +m4 $M4SUPPORT $HEADER_INTERFACES $2 $5/iferror.m4 | sed -e
> > > s/dollarsstar/\$\$\*/g >> $5/all_interfaces.conf
> > > +echo "divert" >> $5/all_interfaces.conf
> > > +# tmp/%.mod
> > > +m4 $M4PARAM -s $M4SUPPORT $5/all_interfaces.conf $1 > $5/$MODULE_NAME.tmp
> > > +/usr/bin/checkmodule -M -m $5/$MODULE_NAME.tmp -o $5/$MODULE_NAME.mod
> > > +# tmp/%.mod.fc
> > > +m4 $M4PARAM $M4SUPPORT $3 > $5/$MODULE_NAME.mod.fc
> > > +# %.pp
> > > +/usr/bin/semodule_package -o $4 -m $5/$MODULE_NAME.mod -f
> > > $5/$MODULE_NAME.mod.fc
>
> Can you change this to use Python, since our strategy is to eliminate
> use of all scripting languages other than Python 3:
>
> https://libvirt.org/strategy.html
>
>
> > > diff --git a/selinux/meson.build b/selinux/meson.build
> > > new file mode 100644
> > > index 00..1c76fd40aa
> > > --- /dev/null
> > > +++ b/selinux/meson.build
> > > @@ -0,0 +1,23 @@
> > >
Re: [PATCH 05/14] migrate: remove QMP/HMP commands for speed, downtime and cache size
* Daniel P. Berrangé (berra...@redhat.com) wrote: > The generic 'migrate_set_parameters' command handle all types of param. > > Only the QMP commands were documented in the deprecations page, but the > rationale for deprecating applies equally to HMP, and the replacements > exist. Furthermore the HMP commands are just shims to the QMP commands, > so removing the latter breaks the former unless they get re-implemented. > > Signed-off-by: Daniel P. Berrangé Yes OK; ouch that's going to break my 7 years of instinctive 'migrate_set_speed 10G' typing, but it's probably the right thing to do. Reviewed-by: Dr. David Alan Gilbert > --- > docs/devel/migration.rst| 2 +- > docs/rdma.txt | 2 +- > docs/system/deprecated.rst | 10 --- > docs/system/removed-features.rst| 20 ++ > docs/xbzrle.txt | 5 -- > hmp-commands-info.hx| 13 > hmp-commands.hx | 45 - > include/monitor/hmp.h | 4 -- > migration/migration.c | 45 - > migration/ram.c | 2 +- > monitor/hmp-cmds.c | 34 -- > qapi/migration.json | 98 - > tests/migration/guestperf/engine.py | 16 ++--- > tests/qemu-iotests/181 | 2 +- > tests/qtest/migration-test.c| 48 -- > tests/qtest/test-hmp.c | 6 +- > tests/qtest/vhost-user-test.c | 8 +-- > 17 files changed, 40 insertions(+), 320 deletions(-) > > diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst > index ad381b89b2..19c3d4f3ea 100644 > --- a/docs/devel/migration.rst > +++ b/docs/devel/migration.rst > @@ -641,7 +641,7 @@ time per vCPU. > > .. note:: >During the postcopy phase, the bandwidth limits set using > - ``migrate_set_speed`` is ignored (to avoid delaying requested pages that > + ``migrate_set_parameter`` is ignored (to avoid delaying requested pages > that >the destination is waiting for). > > Postcopy device transfer > diff --git a/docs/rdma.txt b/docs/rdma.txt > index 49dc9f8bca..2b4cdea1d8 100644 > --- a/docs/rdma.txt > +++ b/docs/rdma.txt > @@ -89,7 +89,7 @@ RUNNING: > First, set the migration speed to match your hardware's capabilities: > > QEMU Monitor Command: > -$ migrate_set_speed 40g # or whatever is the MAX of your RDMA device > +$ migrate_set_parameter max_bandwidth 40g # or whatever is the MAX of your > RDMA device > > Next, on the destination machine, add the following to the QEMU command line: > > diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst > index c577cc97c4..e214f0a9cf 100644 > --- a/docs/system/deprecated.rst > +++ b/docs/system/deprecated.rst > @@ -147,11 +147,6 @@ Use argument ``id`` instead. > > Use argument ``id`` instead. > > -``migrate_set_downtime`` and ``migrate_set_speed`` (since 2.8.0) > - > - > -Use ``migrate-set-parameters`` instead. > - > ``query-named-block-nodes`` result ``encryption_key_missing`` (since 2.10.0) > > > @@ -167,11 +162,6 @@ Always false. > > Use argument value ``null`` instead. > > -``migrate-set-cache-size`` and ``query-migrate-cache-size`` (since 2.11.0) > -'' > - > -Use ``migrate-set-parameters`` and ``query-migrate-parameters`` instead. > - > ``block-commit`` arguments ``base`` and ``top`` (since 3.1.0) > ' > > diff --git a/docs/system/removed-features.rst > b/docs/system/removed-features.rst > index 74d022babf..2c5513dcc7 100644 > --- a/docs/system/removed-features.rst > +++ b/docs/system/removed-features.rst > @@ -85,6 +85,16 @@ Use ``blockdev-change-medium`` or ``change-vnc-password`` > instead. > The ``query-events`` command has been superseded by the more powerful > and accurate ``query-qmp-schema`` command. > > +``migrate_set_cache_size`` and ``query-migrate-cache-size`` (removed in 6.0) > + > + > +Use ``migrate_set_parameter`` and ``info migrate_parameters`` instead. > + > +``migrate_set_downtime`` and ``migrate_set_speed`` (removed in 6.0) > +''' > + > +Use ``migrate_set_parameter`` instead. > + > Human Monitor Protocol (HMP) commands > - > > @@ -113,6 +123,16 @@ The ``acl_show``, ``acl_reset``, ``acl_policy``, > ``acl_add``, and > ``acl_remove`` commands were removed with no replacement. Authorization > for VNC should be performed using the pluggable QAuthZ objects. > > +``migrate-set-cache-size`` and ``info migrate-cache-size`` (removed in 6.0) >
[PATCH v2 2/3] ui: introduce "password-secret" option for SPICE server
Currently when using SPICE the "password" option provides the password
in plain text on the command line. This is insecure as it is visible
to all processes on the host. As an alternative, the password can be
provided separately via the monitor.
This introduces a "password-secret" option which lets the password be
provided up front.
$QEMU --object secret,id=vncsec0,file=passwd.txt \
--spice port=5901,password-secret=vncsec0
Signed-off-by: Daniel P. Berrangé
---
qemu-options.hx | 9 +++--
ui/spice-core.c | 30 --
2 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/qemu-options.hx b/qemu-options.hx
index 722d56eab3..77bb834e37 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1899,7 +1899,8 @@ DEF("spice", HAS_ARG, QEMU_OPTION_spice,
" [,tls-ciphers=]\n"
" [,tls-channel=[main|display|cursor|inputs|record|playback]]\n"
"
[,plaintext-channel=[main|display|cursor|inputs|record|playback]]\n"
-" [,sasl=on|off][,password=][,disable-ticketing=on|off]\n"
+" [,sasl=on|off][,disable-ticketing=on|off]\n"
+" [,password=][,password-secret=]\n"
" [,image-compression=[auto_glz|auto_lz|quic|glz|lz|off]]\n"
" [,jpeg-wan-compression=[auto|never|always]]\n"
" [,zlib-glz-wan-compression=[auto|never|always]]\n"
@@ -1924,9 +1925,13 @@ SRST
``ipv4=on|off``; \ ``ipv6=on|off``; \ ``unix=on|off``
Force using the specified IP version.
-``password=``
+``password=``
Set the password you need to authenticate.
+``password-secret=``
+Set the ID of the ``secret`` object containing the password
+you need to authenticate.
+
``sasl=on|off``
Require that the client use SASL to authenticate with the spice.
The exact choice of authentication method used is controlled
diff --git a/ui/spice-core.c b/ui/spice-core.c
index beee932f55..7f0e005ca9 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -34,6 +34,7 @@
#include "qapi/qapi-events-ui.h"
#include "qemu/notify.h"
#include "qemu/option.h"
+#include "crypto/secret_common.h"
#include "migration/misc.h"
#include "hw/pci/pci_bus.h"
#include "ui/spice-display.h"
@@ -415,6 +416,9 @@ static QemuOptsList qemu_spice_opts = {
},{
.name = "password",
.type = QEMU_OPT_STRING,
+},{
+.name = "password-secret",
+.type = QEMU_OPT_STRING,
},{
.name = "disable-ticketing",
.type = QEMU_OPT_BOOL,
@@ -636,7 +640,9 @@ void qemu_spice_display_init_done(void)
static void qemu_spice_init(void)
{
QemuOpts *opts = QTAILQ_FIRST(_spice_opts.head);
-const char *password, *str, *x509_dir, *addr,
+char *password = NULL;
+const char *passwordSecret;
+const char *str, *x509_dir, *addr,
*x509_key_password = NULL,
*x509_dh_file = NULL,
*tls_ciphers = NULL;
@@ -663,7 +669,26 @@ static void qemu_spice_init(void)
error_report("spice tls-port is out of range");
exit(1);
}
-password = qemu_opt_get(opts, "password");
+passwordSecret = qemu_opt_get(opts, "password-secret");
+if (passwordSecret) {
+Error *local_err = NULL;
+if (qemu_opt_get(opts, "password")) {
+error_report("'password' option is mutually exclusive with "
+ "'password-secret'");
+exit(1);
+}
+password = qcrypto_secret_lookup_as_utf8(passwordSecret,
+ _err);
+if (!password) {
+error_report_err(local_err);
+exit(1);
+}
+} else {
+str = qemu_opt_get(opts, "password");
+if (str) {
+password = g_strdup(str);
+}
+}
if (tls_port) {
x509_dir = qemu_opt_get(opts, "x509-dir");
@@ -809,6 +834,7 @@ static void qemu_spice_init(void)
g_free(x509_key_file);
g_free(x509_cert_file);
g_free(x509_cacert_file);
+g_free(password);
#ifdef HAVE_SPICE_GL
if (qemu_opt_get_bool(opts, "gl", 0)) {
--
2.29.2
[PATCH v2 3/3] ui: deprecate "password" option for SPICE server
With the new "password-secret" option, there is no reason to use the old
inecure "password" option with -spice, so it can be deprecated.
Signed-off-by: Daniel P. Berrangé
---
docs/system/deprecated.rst | 8
qemu-options.hx| 4
ui/spice-core.c| 2 ++
3 files changed, 14 insertions(+)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 241b28a521..e742c8d311 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -166,6 +166,14 @@ Using ``-M kernel-irqchip=off`` with x86 machine types
that include a local
APIC is deprecated. The ``split`` setting is supported, as is using
``-M kernel-irqchip=off`` with the ISA PC machine type.
+``-spice password=string`` (since 6.0)
+''
+
+This option is insecure because the SPICE password remains visible in
+the process listing. This is replaced by the new ``password-secret``
+option which lets the password be securely provided on the command
+line using a ``secret`` object instance.
+
QEMU Machine Protocol (QMP) commands
diff --git a/qemu-options.hx b/qemu-options.hx
index 77bb834e37..48382a8a2a 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1928,6 +1928,10 @@ SRST
``password=``
Set the password you need to authenticate.
+This option is deprecated and insecure because it leaves the
+password visible in the process listing. Use ``password-secret``
+instead.
+
``password-secret=``
Set the ID of the ``secret`` object containing the password
you need to authenticate.
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 7f0e005ca9..235d61f0c1 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -686,6 +686,8 @@ static void qemu_spice_init(void)
} else {
str = qemu_opt_get(opts, "password");
if (str) {
+warn_report("'password' option is deprecated and insecure, "
+"use 'password-secret' instead");
password = g_strdup(str);
}
}
--
2.29.2
[PATCH v2 0/3] ui: add support for 'secret' object to provide VNC/SPICE passwords
This fixes a long standing limitation of the VNC/SPICE code which was unable to securely accept passswords on the CLI, instead requiring use of separate monitor commands after startup. In v2: - Dropped patch removing ACL commands, as it will move to a bigger deprecation cleanup series - Rebased and resolved conflicts Daniel P. Berrangé (3): ui: introduce "password-secret" option for VNC servers ui: introduce "password-secret" option for SPICE server ui: deprecate "password" option for SPICE server docs/system/deprecated.rst | 8 qemu-options.hx| 18 -- ui/spice-core.c| 32 ++-- ui/vnc.c | 23 ++- 4 files changed, 76 insertions(+), 5 deletions(-) -- 2.29.2
[PATCH v2 1/3] ui: introduce "password-secret" option for VNC servers
Currently when using VNC the "password" flag turns on password based
authentication. The actual password has to be provided separately via
the monitor.
This introduces a "password-secret" option which lets the password be
provided up front.
$QEMU --object secret,id=vncsec0,file=passwd.txt \
--vnc localhost:0,password-secret=vncsec0
Signed-off-by: Daniel P. Berrangé
---
qemu-options.hx | 5 +
ui/vnc.c| 23 ++-
2 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/qemu-options.hx b/qemu-options.hx
index 90801286c6..722d56eab3 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2165,6 +2165,11 @@ SRST
time to allow password to expire immediately or never
expire.
+``password-secret=``
+Require that password based authentication is used for client
+connections, using the password provided by the ``secret``
+object identified by ``secret-id``.
+
``tls-creds=ID``
Provides the ID of a set of TLS credentials to use to secure the
VNC server. They will apply to both the normal VNC server socket
diff --git a/ui/vnc.c b/ui/vnc.c
index 310abc9378..e8e3426a65 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -48,6 +48,7 @@
#include "crypto/tlscredsanon.h"
#include "crypto/tlscredsx509.h"
#include "crypto/random.h"
+#include "crypto/secret_common.h"
#include "qom/object_interfaces.h"
#include "qemu/cutils.h"
#include "qemu/help_option.h"
@@ -3459,6 +3460,9 @@ static QemuOptsList qemu_vnc_opts = {
},{
.name = "password",
.type = QEMU_OPT_BOOL,
+},{
+.name = "password-secret",
+.type = QEMU_OPT_STRING,
},{
.name = "reverse",
.type = QEMU_OPT_BOOL,
@@ -3931,6 +3935,7 @@ void vnc_display_open(const char *id, Error **errp)
int lock_key_sync = 1;
int key_delay_ms;
const char *audiodev;
+const char *passwordSecret;
if (!vd) {
error_setg(errp, "VNC display not active");
@@ -3948,7 +3953,23 @@ void vnc_display_open(const char *id, Error **errp)
goto fail;
}
-password = qemu_opt_get_bool(opts, "password", false);
+
+passwordSecret = qemu_opt_get(opts, "password-secret");
+if (passwordSecret) {
+if (qemu_opt_get(opts, "password")) {
+error_setg(errp,
+ "'password' flag is redundant with 'password-secret'");
+goto fail;
+}
+vd->password = qcrypto_secret_lookup_as_utf8(passwordSecret,
+ errp);
+if (!vd->password) {
+goto fail;
+}
+password = true;
+} else {
+password = qemu_opt_get_bool(opts, "password", false);
+}
if (password) {
if (fips_get_state()) {
error_setg(errp,
--
2.29.2
Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
Am 11.03.2021 um 12:24 hat Peter Krempa geschrieben: > On Thu, Mar 11, 2021 at 09:37:11 +0100, Kevin Wolf wrote: > > Am 11.03.2021 um 08:47 hat Peter Krempa geschrieben: > > > On Wed, Mar 10, 2021 at 18:30:44 +0100, Kevin Wolf wrote: > > > > Am 10.03.2021 um 15:31 hat Paolo Bonzini geschrieben: > > > > > On 10/03/21 15:22, Peter Krempa wrote: > > [...] > > > > -object > > > memory-backend-ram,id=ram-node2,size=24578621440,host-nodes=1-2,host-nodes=5,host-nodes=7,policy=bind > > > > Oh, we have ranges, too... That makes the compatibility code even > > nastier to write. I doubt that we can implement this in the keyval > > parser alone without touching the visitor. :-/ > > > > > If any of the above is to be deprecated we'll need to adjust our > > > JSON->commandline generator accordignly. > > > > > > Luckily the 'host-nodes' is storeable as a bitmap and the generator is > > > actually modular to allow plugging an array interpretor which actually > > > does the above conversion from a JSON array. > > > > > > So, what is the preferred syntax here? Additionally we might need a > > > witness property to detect when to use the new syntax if basing it on > > > the object-add qapification will not be enough. > > > > The list syntax supported by the keyval parser is the one you know from > > -blockdev: host-nodes.0=3,host-nodes.1=7, ... > > I think that should be easy enough to convert to. We could also support JSON syntax in QEMU. That would probably be even more convenient for libvirt? > Is it safe to do right away (with the old parser?). Otherwise we need > to agree on something which will let us detect when it's safe to > change. Neither keyval nor JSON syntax work with the old QemuOpts parser. What is the usual way to do this for command line options? If we don't have a good way there, we can always tie it to something in the QAPI schema. If we still get this solved in time for 6.0, we could use the existence of ObjectOptions. Or all else failing, we can use a feature flag. Kevin
Re: [PATCH 3/3] selinux: Remove 'make' dependency
On Wed, Mar 10, 2021 at 01:50:43PM -0500, Neal Gompa wrote:
> On Wed, Mar 10, 2021 at 7:43 AM Nikola Knazekova wrote:
> >
> > From: Vit Mojzis
> >
> > Compile the policy using a shell script executed by meson.
> >
> > Signed-off-by: Vit Mojzis
> > ---
> > libvirt.spec.in | 12
> > meson.build | 12
> > selinux/compile_policy.sh | 39 +++
> > selinux/meson.build | 23 +++
> > 4 files changed, 74 insertions(+), 12 deletions(-)
> > create mode 100755 selinux/compile_policy.sh
> > create mode 100644 selinux/meson.build
> >
> > diff --git a/libvirt.spec.in b/libvirt.spec.in
> > index db08d91043..de664084fa 100644
> > --- a/libvirt.spec.in
> > +++ b/libvirt.spec.in
> > @@ -1240,14 +1240,6 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y'
> > %{_specdir}/%{name}.spec)
> > %{?arg_login_shell}
> >
> > %meson_build
> > -%if 0%{?with_selinux}
> > -# SELinux policy (originally from selinux-policy-contrib)
> > -# this policy module will override the production module
> > -cd selinux
> > -
> > -make -f %{_datadir}/selinux/devel/Makefile %{modulename}.pp
> > -bzip2 -9 %{modulename}.pp
> > -%endif
> >
> > %install
> > rm -fr %{buildroot}
> > @@ -1332,10 +1324,6 @@ mv
> > $RPM_BUILD_ROOT%{_datadir}/systemtap/tapset/libvirt_qemu_probes.stp \
> > %endif
> > %endif
> >
> > -%if 0%{?with_selinux}
> > -install -D -m 0644 selinux/%{modulename}.pp.bz2
> > %{buildroot}%{_datadir}/selinux/packages/%{selinuxtype}/%{modulename}.pp.bz2
> > -%endif
> > -
> > %check
> > # Building on slow archs, like emulated s390x in Fedora copr, requires
> > # raising the test timeout
> > diff --git a/meson.build b/meson.build
> > index c81c6ab205..d060e441b5 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -2183,6 +2183,18 @@ endif
> >
> > subdir('build-aux')
> >
> > +os_release = run_command('grep', '^ID=', '/etc/os-release').stdout()
> > +os_version = run_command('grep', '^VERSION_ID=',
> > '/etc/os-release').stdout().split('=')
> > +if (os_version.length() == 2)
> > + os_version = os_version[1]
> > +else
> > + os_version = 0
> > +endif
> > +
> > +if ((os_release.contains('fedora') and os_version.version_compare('>32'))
> > or
> > +(os_release.contains('rhel') and os_version.version_compare('>7')))
> > + subdir('selinux')
> > +endif
> >
> > # install pkgconfig files
> > pkgconfig_files = [
> > diff --git a/selinux/compile_policy.sh b/selinux/compile_policy.sh
> > new file mode 100755
> > index 00..02780e4aed
> > --- /dev/null
> > +++ b/selinux/compile_policy.sh
> > @@ -0,0 +1,39 @@
> > +#!/bin/sh
> > +set -x
> > +
> > +if [[ $# -ne 5 ]] ; then
> > +echo "Usage: compile_policy.sh .te .if .fc
> > .pp "
> > +exit 1
> > +fi
> > +
> > +# checkmodule requires consistent file names
> > +MODULE_NAME=$(basename -- "$1")
> > +MODULE_NAME=${MODULE_NAME%.*}
> > +
> > +M4PARAM="-D enable_mcs -D distro_redhat -D hide_broken_symptoms -D
> > mls_num_sens=16 -D mls_num_cats=1024 -D mcs_num_cats=1024"
> > +SHAREDIR="/usr/share/selinux"
> > +HEADERDIR="$SHAREDIR/devel/include"
> > +M4SUPPORT=$(echo $HEADERDIR/support/*.spt)
> > +HEADER_LAYERS=$(find "/usr/share/selinux/devel/include"/* -maxdepth 0
> > -type d | grep -v "/usr/share/selinux/devel/include/support")
> > +HEADER_INTERFACES=""
> > +for LAYER in $HEADER_LAYERS
> > +do
> > +HEADER_INTERFACES="$HEADER_INTERFACES $(echo $LAYER/*.if)"
> > +done
> > +
> > +# prepare temp folder
> > +mkdir -p $5
> > +# remove old trash from the temp folder
> > +rm -rf "$5/iferror.m4 $5/all_interfaces.conf $5/$MODULE_NAME.*"
> > +# tmp/all_interfaces.conf
> > +echo "ifdef(\`__if_error',\`m4exit(1)')" > $5/iferror.m4
> > +echo "divert(-1)" > $5/all_interfaces.conf
> > +m4 $M4SUPPORT $HEADER_INTERFACES $2 $5/iferror.m4 | sed -e
> > s/dollarsstar/\$\$\*/g >> $5/all_interfaces.conf
> > +echo "divert" >> $5/all_interfaces.conf
> > +# tmp/%.mod
> > +m4 $M4PARAM -s $M4SUPPORT $5/all_interfaces.conf $1 > $5/$MODULE_NAME.tmp
> > +/usr/bin/checkmodule -M -m $5/$MODULE_NAME.tmp -o $5/$MODULE_NAME.mod
> > +# tmp/%.mod.fc
> > +m4 $M4PARAM $M4SUPPORT $3 > $5/$MODULE_NAME.mod.fc
> > +# %.pp
> > +/usr/bin/semodule_package -o $4 -m $5/$MODULE_NAME.mod -f
> > $5/$MODULE_NAME.mod.fc
Can you change this to use Python, since our strategy is to eliminate
use of all scripting languages other than Python 3:
https://libvirt.org/strategy.html
> > diff --git a/selinux/meson.build b/selinux/meson.build
> > new file mode 100644
> > index 00..1c76fd40aa
> > --- /dev/null
> > +++ b/selinux/meson.build
> > @@ -0,0 +1,23 @@
> > +selinux_sources = [
> > + 'virt.te',
> > + 'virt.if',
> > + 'virt.fc',
> > +]
> > +
> > +compile_policy_prog = find_program('compile_policy.sh')
> > +
> > +virt_pp = custom_target('virt.pp',
> > + output : 'virt.pp',
> > + input : selinux_sources,
> > + command : [compile_policy_prog, '@INPUT@', '@OUTPUT@', 'selinux/tmp'],
> > +
Re: [PATCH 3/3] selinux: Remove 'make' dependency
On 3/10/21 7:50 PM, Neal Gompa wrote:
On Wed, Mar 10, 2021 at 7:43 AM Nikola Knazekova wrote:
From: Vit Mojzis
Compile the policy using a shell script executed by meson.
Signed-off-by: Vit Mojzis
---
libvirt.spec.in | 12
meson.build | 12
selinux/compile_policy.sh | 39 +++
selinux/meson.build | 23 +++
4 files changed, 74 insertions(+), 12 deletions(-)
create mode 100755 selinux/compile_policy.sh
create mode 100644 selinux/meson.build
diff --git a/libvirt.spec.in b/libvirt.spec.in
index db08d91043..de664084fa 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1240,14 +1240,6 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y'
%{_specdir}/%{name}.spec)
%{?arg_login_shell}
%meson_build
-%if 0%{?with_selinux}
-# SELinux policy (originally from selinux-policy-contrib)
-# this policy module will override the production module
-cd selinux
-
-make -f %{_datadir}/selinux/devel/Makefile %{modulename}.pp
-bzip2 -9 %{modulename}.pp
-%endif
%install
rm -fr %{buildroot}
@@ -1332,10 +1324,6 @@ mv
$RPM_BUILD_ROOT%{_datadir}/systemtap/tapset/libvirt_qemu_probes.stp \
%endif
%endif
-%if 0%{?with_selinux}
-install -D -m 0644 selinux/%{modulename}.pp.bz2
%{buildroot}%{_datadir}/selinux/packages/%{selinuxtype}/%{modulename}.pp.bz2
-%endif
-
%check
# Building on slow archs, like emulated s390x in Fedora copr, requires
# raising the test timeout
diff --git a/meson.build b/meson.build
index c81c6ab205..d060e441b5 100644
--- a/meson.build
+++ b/meson.build
@@ -2183,6 +2183,18 @@ endif
subdir('build-aux')
+os_release = run_command('grep', '^ID=', '/etc/os-release').stdout()
+os_version = run_command('grep', '^VERSION_ID=',
'/etc/os-release').stdout().split('=')
+if (os_version.length() == 2)
+ os_version = os_version[1]
+else
+ os_version = 0
+endif
+
+if ((os_release.contains('fedora') and os_version.version_compare('>32')) or
+(os_release.contains('rhel') and os_version.version_compare('>7')))
+ subdir('selinux')
+endif
# install pkgconfig files
pkgconfig_files = [
diff --git a/selinux/compile_policy.sh b/selinux/compile_policy.sh
new file mode 100755
index 00..02780e4aed
--- /dev/null
+++ b/selinux/compile_policy.sh
@@ -0,0 +1,39 @@
+#!/bin/sh
+set -x
+
+if [[ $# -ne 5 ]] ; then
+echo "Usage: compile_policy.sh .te .if .fc .pp
"
+exit 1
+fi
+
+# checkmodule requires consistent file names
+MODULE_NAME=$(basename -- "$1")
+MODULE_NAME=${MODULE_NAME%.*}
+
+M4PARAM="-D enable_mcs -D distro_redhat -D hide_broken_symptoms -D mls_num_sens=16
-D mls_num_cats=1024 -D mcs_num_cats=1024"
+SHAREDIR="/usr/share/selinux"
+HEADERDIR="$SHAREDIR/devel/include"
+M4SUPPORT=$(echo $HEADERDIR/support/*.spt)
+HEADER_LAYERS=$(find "/usr/share/selinux/devel/include"/* -maxdepth 0 -type d | grep -v
"/usr/share/selinux/devel/include/support")
+HEADER_INTERFACES=""
+for LAYER in $HEADER_LAYERS
+do
+HEADER_INTERFACES="$HEADER_INTERFACES $(echo $LAYER/*.if)"
+done
+
+# prepare temp folder
+mkdir -p $5
+# remove old trash from the temp folder
+rm -rf "$5/iferror.m4 $5/all_interfaces.conf $5/$MODULE_NAME.*"
+# tmp/all_interfaces.conf
+echo "ifdef(\`__if_error',\`m4exit(1)')" > $5/iferror.m4
+echo "divert(-1)" > $5/all_interfaces.conf
+m4 $M4SUPPORT $HEADER_INTERFACES $2 $5/iferror.m4 | sed -e s/dollarsstar/\$\$\*/g
>> $5/all_interfaces.conf
+echo "divert" >> $5/all_interfaces.conf
+# tmp/%.mod
+m4 $M4PARAM -s $M4SUPPORT $5/all_interfaces.conf $1 > $5/$MODULE_NAME.tmp
+/usr/bin/checkmodule -M -m $5/$MODULE_NAME.tmp -o $5/$MODULE_NAME.mod
+# tmp/%.mod.fc
+m4 $M4PARAM $M4SUPPORT $3 > $5/$MODULE_NAME.mod.fc
+# %.pp
+/usr/bin/semodule_package -o $4 -m $5/$MODULE_NAME.mod -f
$5/$MODULE_NAME.mod.fc
diff --git a/selinux/meson.build b/selinux/meson.build
new file mode 100644
index 00..1c76fd40aa
--- /dev/null
+++ b/selinux/meson.build
@@ -0,0 +1,23 @@
+selinux_sources = [
+ 'virt.te',
+ 'virt.if',
+ 'virt.fc',
+]
+
+compile_policy_prog = find_program('compile_policy.sh')
+
+virt_pp = custom_target('virt.pp',
+ output : 'virt.pp',
+ input : selinux_sources,
+ command : [compile_policy_prog, '@INPUT@', '@OUTPUT@', 'selinux/tmp'],
+ install : false)
+
+bzip2_prog = find_program('bzip2')
+
+bzip = custom_target('virt.pp.bz2',
+ output : 'virt.pp.bz2',
+ input : virt_pp,
+ command : [bzip2_prog, '-c', '-9', '@INPUT@'],
+ capture : true,
+ install : true,
+ install_dir : 'share/selinux/packages/targeted')
--
2.29.2
This smells like a bad idea, because we're not relying on the
framework that SELinux policies are supposed to be built with. I don't
think we should do this.
Hi,
not sure what you mean. The shell script is almost a 1 to 1 copy of the
original Makefile from selinux-policy-devel so it should not cause any
issues.
If you mean the whole idea of moving the policy from selinux-policy
packages to libvirt, than this has proven
Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
On Thu, Mar 11, 2021 at 09:37:11 +0100, Kevin Wolf wrote: > Am 11.03.2021 um 08:47 hat Peter Krempa geschrieben: > > On Wed, Mar 10, 2021 at 18:30:44 +0100, Kevin Wolf wrote: > > > Am 10.03.2021 um 15:31 hat Paolo Bonzini geschrieben: > > > > On 10/03/21 15:22, Peter Krempa wrote: [...] > > -object > > memory-backend-ram,id=ram-node2,size=24578621440,host-nodes=1-2,host-nodes=5,host-nodes=7,policy=bind > > Oh, we have ranges, too... That makes the compatibility code even > nastier to write. I doubt that we can implement this in the keyval > parser alone without touching the visitor. :-/ > > > If any of the above is to be deprecated we'll need to adjust our > > JSON->commandline generator accordignly. > > > > Luckily the 'host-nodes' is storeable as a bitmap and the generator is > > actually modular to allow plugging an array interpretor which actually > > does the above conversion from a JSON array. > > > > So, what is the preferred syntax here? Additionally we might need a > > witness property to detect when to use the new syntax if basing it on > > the object-add qapification will not be enough. > > The list syntax supported by the keyval parser is the one you know from > -blockdev: host-nodes.0=3,host-nodes.1=7, ... I think that should be easy enough to convert to. Is it safe to do right away (with the old parser?). Otherwise we need to agree on something which will let us detect when it's safe to change.
Re: [PATCH 0/4] ui: add support for 'secret' object to provide VNC/SPICE passwords
On Thu, Mar 11, 2021 at 12:13:42PM +0100, Gerd Hoffmann wrote: > On Thu, Mar 11, 2021 at 10:37:45AM +, Daniel P. Berrangé wrote: > > Ping > > Looks good but doesn't apply cleanly, can you rebase? > > (current ui queue is gitlab.com/kraxel/qemu queue/ui, there are no > spice/vnc changes queued so it probably doesn't make a difference > compared to latest master) Sure, will rebase to master and check it also applies to this queue cleanly. 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 :|
Re: [PATCH 0/4] ui: add support for 'secret' object to provide VNC/SPICE passwords
On Thu, Mar 11, 2021 at 10:37:45AM +, Daniel P. Berrangé wrote: > Ping Looks good but doesn't apply cleanly, can you rebase? (current ui queue is gitlab.com/kraxel/qemu queue/ui, there are no spice/vnc changes queued so it probably doesn't make a difference compared to latest master) take care, Gerd
Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
On 11/03/21 11:38, Markus Armbruster wrote:
Here's a differently terrible hack. We have
keyval_parse() visitor
optarg > QObject > QAPI type
Idea: hack the QObject. If we're working for -object, and QObject maps
key "qom-type" to value "memory-backend-ram", get the value of
host-nodes, and if it's a string, parse it into a list like the opts
visitor does, and put that back, replacing the string value.
Same for other uses of Memdev and NumaNodeOptions with -object, if they
exist.
This doesn't help with backwards compatibility, since keyval loses the
first of host-nodes=0,host-nodes=4.
I would rather keep the OptsVisitor here. Do the same check for JSON
syntax that you have in qobject_input_visitor_new_str, and whenever you
need to walk all -object arguments, use something like this:
typedef struct ObjectArgument {
const char *id;
QDict *json;/* or NULL for QemuOpts */
QSIMPLEQ_ENTRY(ObjectArgument) next;
}
I already had patches in my queue to store -object in a GSList of
dictionaries, changing it to use the above is easy enough.
Paolo
Re: [PATCH 02/23] qemu: factor out qemuValidateDomainBlkdeviotune
ср, 3 мар. 2021 г. в 15:54, Peter Krempa :
> On Mon, Jan 11, 2021 at 12:49:55 +0300, Nikolay Shirokovskiy wrote:
> > It can also be used for validation of input in qemuDomainSetBlockIoTune.
> >
> > Signed-off-by: Nikolay Shirokovskiy
> > ---
> > src/qemu/qemu_validate.c | 100
> ++-
> > src/qemu/qemu_validate.h | 4 ++
> > 2 files changed, 60 insertions(+), 44 deletions(-)
> >
> > diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> > index eadf3af..6a27565 100644
> > --- a/src/qemu/qemu_validate.c
> > +++ b/src/qemu/qemu_validate.c
> > @@ -2696,6 +2696,61 @@ qemuValidateDomainDeviceDefDiskFrontend(const
> virDomainDiskDef *disk,
> > }
> >
> >
> > +int
> > +qemuValidateDomainBlkdeviotune(const virDomainBlockIoTuneInfo *iotune,
> > + virQEMUCapsPtr qemuCaps)
> > +{
>
> The check that group_name must be set along with other fields :
>
> /* group_name by itself is ignored by qemu */
> if (disk->blkdeviotune.group_name &&
> !virDomainBlockIoTuneInfoHasAny(>blkdeviotune)) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>_("group_name can be configured only together with "
> "settings"));
> return -1;
> }
>
>
> also belongs here.
>
I cannot put this check into qemuValidateDomainBlkdeviotune. The thing is
I'm going to reuse this function
in qemuDomainSetBlockIoTune and in qemuDomainSetBlockIoTune is it fine to
have group_name only in input.
This means "move this disk to that io tune group" or "create io tune group
and put this disk into it" depending
on whether io tune with that name exists or not (this is what
qemuDomainSetBlockIoTuneDefaults do).
Nikolay
>
>
> > +if (iotune->total_bytes_sec > QEMU_BLOCK_IOTUNE_MAX ||
> > +iotune->read_bytes_sec > QEMU_BLOCK_IOTUNE_MAX ||
> > +iotune->write_bytes_sec > QEMU_BLOCK_IOTUNE_MAX ||
> > +iotune->total_iops_sec > QEMU_BLOCK_IOTUNE_MAX ||
> > +iotune->read_iops_sec > QEMU_BLOCK_IOTUNE_MAX ||
> > +iotune->write_iops_sec > QEMU_BLOCK_IOTUNE_MAX ||
> > +iotune->total_bytes_sec_max > QEMU_BLOCK_IOTUNE_MAX ||
> > +iotune->read_bytes_sec_max > QEMU_BLOCK_IOTUNE_MAX ||
> > +iotune->write_bytes_sec_max > QEMU_BLOCK_IOTUNE_MAX ||
> > +iotune->total_iops_sec_max > QEMU_BLOCK_IOTUNE_MAX ||
> > +iotune->read_iops_sec_max > QEMU_BLOCK_IOTUNE_MAX ||
> > +iotune->write_iops_sec_max > QEMU_BLOCK_IOTUNE_MAX ||
> > +iotune->size_iops_sec > QEMU_BLOCK_IOTUNE_MAX) {
> > +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
> > + _("block I/O throttle limit must "
> > + "be no more than %llu using QEMU"),
> > + QEMU_BLOCK_IOTUNE_MAX);
> > +return -1;
> > +}
>
> We also nowadays prefer if the error detail strings are not broken up,
> but that's not a required change.
>
> With the group name check moved too:
>
> Reviewed-by: Peter Krempa
>
>
Re: [PATCH 10/14] hw/scsi: remove 'scsi-disk' device
On Wed, Feb 24, 2021 at 03:26:59PM +0100, Thomas Huth wrote:
> On 24/02/2021 14.11, Daniel P. Berrangé wrote:
> > The 'scsi-hd' and 'scsi-cd' devices provide suitable alternatives.
> >
> > Signed-off-by: Daniel P. Berrangé
> > ---
> > docs/system/deprecated.rst | 9 -
> > docs/system/removed-features.rst | 6
> > hw/i386/pc.c | 1 -
> > hw/scsi/scsi-disk.c | 62
> > hw/sparc64/sun4u.c | 1 -
> > scripts/device-crash-test| 1 -
> > tests/qemu-iotests/051 | 2 --
> > tests/qemu-iotests/051.pc.out| 10 --
> > 8 files changed, 6 insertions(+), 86 deletions(-)
>
> I see some occurrances of "scsi-disk" in the config files in the
> docs/config/ directory, too ... I guess they should also be removed?
Those aren't referring to the "scsi-disk" device type, they are qdev
IDs, and do indeed already use "scsi-hd" as the device type.
>
> > diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> > index d7c27144ba..cda7df36e3 100644
> > --- a/hw/sparc64/sun4u.c
> > +++ b/hw/sparc64/sun4u.c
> > @@ -749,7 +749,6 @@ static char *sun4u_fw_dev_path(FWPathProvider *p,
> > BusState *bus,
> > DeviceState *dev)
> > {
> > PCIDevice *pci;
> > -int bus_id;
> > if (!strcmp(object_get_typename(OBJECT(dev)), "pbm-bridge")) {
> > pci = PCI_DEVICE(dev);
>
> This lonely hunk should be squashed into the previous (ide-disk) patch
> instead.
>
> Thomas
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 :|
Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
Kevin Wolf writes:
> Am 10.03.2021 um 15:31 hat Paolo Bonzini geschrieben:
>> On 10/03/21 15:22, Peter Krempa wrote:
>> > I've stumbled upon a regression with this patchset applied:
>> >
>> > error: internal error: process exited while connecting to monitor:
>> > qemu-system-x86_64: -object
>> > memory-backend-ram,id=pc.ram,size=1048576000,host-nodes=0,policy=bind:
>> > Invalid parameter type for 'host-nodes', expected: array
>>
>> This is the magic conversion of "string containing a list of integers"
>> to "list of integers".
>
> Ah, crap. This one wouldn't have been a problem when converting only
> object-add, and I trusted your audit that user creatable objects don't
> depend on any QemuOpts magic. I should have noticed it, too, of course,
> but during the convertion I didn't have QemuOpts in mind, only QOM and
> QAPI.
>
> I checked all object types, and it seems this is the only one that is
> affected. We have a second list in AuthZListProperties, but it contains
> structs, so it was never supported on the command line anyway.
>
>> The relevant code is in qapi/string-input-visitor.c, but I'm not sure where
>> to stick it in the keyval-based parsing flow (i.e.
>> qobject_input_visitor_new_keyval). Markus, any ideas?
>
> The best I can think of at the moment would be adding a flag to the
> keyval parser that would enable the feature only for -object (and only
> in the system emulator, because memory-backend-ram doesn't exist in the
> tools):
>
> The keyval parser would create a list if multiple values are given for
> the same key. Some care needs to be taken to avoid mixing the magic
> list feature with the normal indexed list options.
You're cursing^Wtalking about the wrong list hack, I'm afraid.
QemuOpts can indeed be used in a way so that key=val1,key=val2,... get
collected into a list val1, val2, ... For an example, see how
qemu_semihosting_config_opts() processes the option argument of
-semihosting: repeated arg=... get collected in array
semihosting.argv[].
QOM property "host-nodes" employs a different hack. The QAPI schema
defines it as
{ 'struct': 'Memdev',
'data': {
...
'host-nodes': ['uint16'],
... }}
The QObject input visitor treats it like any other list. To get node 0
and 4, you say
"host-nodes": [0,4]
with its JSON flavor, and
host-nodes.0=0,host-nodes.1=4
with its dotted keys flavor.
The string input visitor and the opts visitor only support *scalar*
values, *except* they both have a hack to support lists of small
integers.
With the opts visitor, saying
host-nodes=0,host-nodes=4
gets you node 0 and 4, and both
host-nodes=0,host-nodes=1
host-nodes=0-1
gets you nodes 0 and 1. This is what parses -object.
Setting NumaNode member @cpus via -numa cpus=... is another user of this
hack.
With the string input visitor, repeating the key doesn't work (there is
no syntax for keys, in fact), but comma does. This is what parses
-global and HMP qom-set.
The problem Peter reported is that switching -object from QemuOpts +
opts visitor to keyval_parse() + QObject input visitor loses the opts
visitor's list-of-integers hack.
The obvious solution is to transplant the hack to the QObject input
visitor. "Ich kann nicht soviel fressen wie ich kotzen möchte" (okay, I
better don't translate this; all you need to know is that I find the
idea utterly disgusting).
There is the more general problem of human-friendly syntax support.
QAPI/QMP eschew encoding complex data in strings. They want you to use
complex data types.
Fine for QMP, machines are generally better off with formatting /
parsing verbose JSON than with formatting / parsing lots of ad hoc
little languages.
Not always fine for humans. Case in point:
host-nodes.0=0,host-nodes.1=4 is decidedly inferior to something like
host-nodes=0+4[*].
Perhaps we need to provide means to define ad hoc little languages for
human use. Specify the parsing function to use for human input in the
QAPI schema.
Doesn't really help us now, because we can't pull it off in time for the
soft freeze.
Here's a differently terrible hack. We have
keyval_parse() visitor
optarg > QObject > QAPI type
Idea: hack the QObject. If we're working for -object, and QObject maps
key "qom-type" to value "memory-backend-ram", get the value of
host-nodes, and if it's a string, parse it into a list like the opts
visitor does, and put that back, replacing the string value.
Same for other uses of Memdev and NumaNodeOptions with -object, if they
exist.
I told you it's terrible :)
[...]
[*] 0+4 and not 0,4 because ',' would have to be doubled in dotted key
syntax.
Re: [PATCH 0/4] ui: add support for 'secret' object to provide VNC/SPICE passwords
Ping On Fri, Feb 19, 2021 at 06:45:52PM +, Daniel P. Berrangé wrote: > This fixes a long standing limitation of the VNC/SPICE code which was > unable to securely accept passswords on the CLI, instead requiring use > of separate monitor commands after startup. > > This takes the opportunity to also remove previously deprecated ACL > functionality from VNC. > > Daniel P. Berrangé (4): > ui: introduce "password-secret" option for VNC servers > ui: introduce "password-secret" option for SPICE server > ui: deprecate "password" option for SPICE server > ui, monitor: remove deprecated VNC ACL option and HMP commands > > docs/system/deprecated.rst | 24 ++-- > docs/system/removed-features.rst | 13 +++ > hmp-commands.hx | 76 - > monitor/misc.c | 187 --- > qemu-options.hx | 17 ++- > ui/spice-core.c | 32 +- > ui/vnc.c | 61 -- > 7 files changed, 88 insertions(+), 322 deletions(-) > > -- > 2.29.2 > 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 :|
[PATCH] virConfLoadConfig: Refactor cleanup
Switch to using the 'g_auto*' helpers.
Signed-off-by: Yi Li
---
src/util/virconf.c | 19 ++-
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/src/util/virconf.c b/src/util/virconf.c
index 16107bce96..11046a1d45 100644
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -1505,26 +1505,19 @@ virConfLoadConfigPath(const char *name)
int
virConfLoadConfig(virConfPtr *conf, const char *name)
{
-char *path = NULL;
-int ret = -1;
+g_autofree char *path = NULL;
*conf = NULL;
if (!(path = virConfLoadConfigPath(name)))
-goto cleanup;
+return -1;
-if (!virFileExists(path)) {
-ret = 0;
-goto cleanup;
-}
+if (!virFileExists(path))
+return 0;
VIR_DEBUG("Loading config file '%s'", path);
if (!(*conf = virConfReadFile(path, 0)))
-goto cleanup;
-
-ret = 0;
+return -1;
- cleanup:
-VIR_FREE(path);
-return ret;
+return 0;
}
--
2.25.3
Re: [PATCH v4 4/4] blockdev: Drop deprecated bogus -drive interface type
On Thu, Mar 11, 2021 at 08:52:21AM +0100, Markus Armbruster wrote: > Drop the crap deprecated in commit a1b40bda08 "blockdev: Deprecate > -drive with bogus interface type" (v5.1.0). > > drive_check_orphaned() no longer depends on qemu_create_cli_devices(). > Call it right after board initialization for clarity. > > Signed-off-by: Markus Armbruster > --- > docs/system/deprecated.rst | 7 -- > docs/system/removed-features.rst | 7 ++ > include/sysemu/blockdev.h| 1 - > blockdev.c | 37 +--- > softmmu/vl.c | 11 +- > 5 files changed, 23 insertions(+), 40 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 :|
Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
On 11/03/21 09:45, Kevin Wolf wrote: I think it's only patch 29 and 30 that we would have to drop, actually. Unfortunately, that still removes one of the most immediately useful features, which is non-scalar properties for -object in the system emulator. But of course, a lot better than not merging it at all. Who is going to include this series in the next pull request, Markus or myself? The time is ticking for soft freeze. QOM is probably the right subsystem, so that would be you. Or I can just merge it myself as long as everyone is fine with it. Eric has some minor comments that I think could be addressed while applying the series. Or should I send a v4 for that (and for dropping patches 29 and 30)? I'd say just send a pull request. :) Paolo
Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
Am 11.03.2021 um 09:14 hat Paolo Bonzini geschrieben: > On 10/03/21 18:30, Kevin Wolf wrote: > > Am 10.03.2021 um 15:31 hat Paolo Bonzini geschrieben: > > > On 10/03/21 15:22, Peter Krempa wrote: > > > > I've stumbled upon a regression with this patchset applied: > > > > > > > > error: internal error: process exited while connecting to monitor: > > > > qemu-system-x86_64: -object > > > > memory-backend-ram,id=pc.ram,size=1048576000,host-nodes=0,policy=bind: > > > > Invalid parameter type for 'host-nodes', expected: array > > > > > > This is the magic conversion of "string containing a list of integers" > > > to "list of integers". > > > > Ah, crap. This one wouldn't have been a problem when converting only > > object-add, and I trusted your audit that user creatable objects don't > > depend on any QemuOpts magic. I should have noticed it, too, of course, > > but during the convertion I didn't have QemuOpts in mind, only QOM and > > QAPI. > > Yeah, let's just drop the -object conversion for now. It will just remove a > few patches. I think it's only patch 29 and 30 that we would have to drop, actually. Unfortunately, that still removes one of the most immediately useful features, which is non-scalar properties for -object in the system emulator. But of course, a lot better than not merging it at all. > Who is going to include this series in the next pull request, Markus or > myself? The time is ticking for soft freeze. QOM is probably the right subsystem, so that would be you. Or I can just merge it myself as long as everyone is fine with it. Eric has some minor comments that I think could be addressed while applying the series. Or should I send a v4 for that (and for dropping patches 29 and 30)? Kevin
Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
Am 11.03.2021 um 08:47 hat Peter Krempa geschrieben: > On Wed, Mar 10, 2021 at 18:30:44 +0100, Kevin Wolf wrote: > > Am 10.03.2021 um 15:31 hat Paolo Bonzini geschrieben: > > > On 10/03/21 15:22, Peter Krempa wrote: > > [...] > > > The keyval parser would create a list if multiple values are given for > > the same key. Some care needs to be taken to avoid mixing the magic > > list feature with the normal indexed list options. > > > > The QAPI schema would then use an alternate between 'int' and ['int'], > > with the the memory-backend-ram implementation changed accordingly. > > > > We could consider immediately deprecating the syntax and printing a > > warning in the keyval parser when it automatically creates a list from > > two values for a key, so that users don't start using this syntax > > By 'creating a list from two values for a key' you mean: > > host-nodes=0,host-nodes=1 > > to be converted into [0, 1] ? > > > instead of the normal list syntax in other places. We'd probably still > > leave the implementation around for -device and other users of the same > > magic. > > There's three options actually that libvirt uses, visible in one our > test files [1] > > For a single value we format: > > -object > memory-backend-ram,id=ram-node0,size=20971520,host-nodes=3,policy=preferred > > For a contiguous list: > > -object > memory-backend-ram,id=ram-node1,size=676331520,host-nodes=0-7,policy=bind > > And for an interleaved list: > > -object > memory-backend-ram,id=ram-node2,size=24578621440,host-nodes=1-2,host-nodes=5,host-nodes=7,policy=bind Oh, we have ranges, too... That makes the compatibility code even nastier to write. I doubt that we can implement this in the keyval parser alone without touching the visitor. :-/ > If any of the above is to be deprecated we'll need to adjust our > JSON->commandline generator accordignly. > > Luckily the 'host-nodes' is storeable as a bitmap and the generator is > actually modular to allow plugging an array interpretor which actually > does the above conversion from a JSON array. > > So, what is the preferred syntax here? Additionally we might need a > witness property to detect when to use the new syntax if basing it on > the object-add qapification will not be enough. The list syntax supported by the keyval parser is the one you know from -blockdev: host-nodes.0=3,host-nodes.1=7, ... Kevin
Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
On 11/03/21 08:47, Peter Krempa wrote: And for an interleaved list: -object memory-backend-ram,id=ram-node2,size=24578621440,host-nodes=1-2,host-nodes=5,host-nodes=7,policy=bind Uhm, I doubt this works? I would have expected "host-nodes=1-2,,5,,7" instead. Paolo
Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
On 10/03/21 18:30, Kevin Wolf wrote: Am 10.03.2021 um 15:31 hat Paolo Bonzini geschrieben: On 10/03/21 15:22, Peter Krempa wrote: I've stumbled upon a regression with this patchset applied: error: internal error: process exited while connecting to monitor: qemu-system-x86_64: -object memory-backend-ram,id=pc.ram,size=1048576000,host-nodes=0,policy=bind: Invalid parameter type for 'host-nodes', expected: array This is the magic conversion of "string containing a list of integers" to "list of integers". Ah, crap. This one wouldn't have been a problem when converting only object-add, and I trusted your audit that user creatable objects don't depend on any QemuOpts magic. I should have noticed it, too, of course, but during the convertion I didn't have QemuOpts in mind, only QOM and QAPI. Yeah, let's just drop the -object conversion for now. It will just remove a few patches. Who is going to include this series in the next pull request, Markus or myself? The time is ticking for soft freeze. Paolo I checked all object types, and it seems this is the only one that is affected. We have a second list in AuthZListProperties, but it contains structs, so it was never supported on the command line anyway. The relevant code is in qapi/string-input-visitor.c, but I'm not sure where to stick it in the keyval-based parsing flow (i.e. qobject_input_visitor_new_keyval). Markus, any ideas? The best I can think of at the moment would be adding a flag to the keyval parser that would enable the feature only for -object (and only in the system emulator, because memory-backend-ram doesn't exist in the tools): The keyval parser would create a list if multiple values are given for the same key. Some care needs to be taken to avoid mixing the magic list feature with the normal indexed list options. The QAPI schema would then use an alternate between 'int' and ['int'], with the the memory-backend-ram implementation changed accordingly. We could consider immediately deprecating the syntax and printing a warning in the keyval parser when it automatically creates a list from two values for a key, so that users don't start using this syntax instead of the normal list syntax in other places. We'd probably still leave the implementation around for -device and other users of the same magic. Kevin
