[PATCH v4 0/4] Drop deprecated floppy config & bogus -drive if=T
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(-) -- 2.26.2
[PATCH v4 2/4] fdc: Drop deprecated floppy configuration
Drop the crap deprecated in commit 4a27a638e7 "fdc: Deprecate
configuring floppies with -global isa-fdc" (v5.1.0).
Signed-off-by: Markus Armbruster
Reviewed-by: Daniel P. Berrangé
---
docs/system/deprecated.rst | 49 ---
docs/system/removed-features.rst | 49 +++
hw/block/fdc.c | 54 +--
tests/qemu-iotests/172 | 31 +-
tests/qemu-iotests/172.out | 562 +--
5 files changed, 53 insertions(+), 692 deletions(-)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index f272c3a414..8d43b9336a 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -94,55 +94,6 @@ QEMU 5.1 has three options:
to the user to load all the images they need.
3. ``-bios `` - Tells QEMU to load the specified file as the firmwrae.
-Floppy controllers' drive properties (since 5.1)
-
-
-Use ``-device floppy,...`` instead. When configuring onboard floppy
-controllers
-::
-
--global isa-fdc.driveA=...
--global sysbus-fdc.driveA=...
--global SUNW,fdtwo.drive=...
-
-become
-::
-
--device floppy,unit=0,drive=...
-
-and
-::
-
--global isa-fdc.driveB=...
--global sysbus-fdc.driveB=...
-
-become
-::
-
--device floppy,unit=1,drive=...
-
-When plugging in a floppy controller
-::
-
--device isa-fdc,...,driveA=...
-
-becomes
-::
-
--device isa-fdc,...
--device floppy,unit=0,drive=...
-
-and
-::
-
--device isa-fdc,...,driveB=...
-
-becomes
-::
-
--device isa-fdc,...
--device floppy,unit=1,drive=...
-
``-drive`` with bogus interface type (since 5.1)
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index 4dcf4f924c..d10fc8cc17 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -38,6 +38,55 @@ or ``-display default,show-cursor=on`` instead.
QEMU 5.0 introduced an alternative syntax to specify the size of the
translation
block cache, ``-accel tcg,tb-size=``.
+Floppy controllers' drive properties (removed in 6.0)
+'
+
+Use ``-device floppy,...`` instead. When configuring onboard floppy
+controllers
+::
+
+-global isa-fdc.driveA=...
+-global sysbus-fdc.driveA=...
+-global SUNW,fdtwo.drive=...
+
+become
+::
+
+-device floppy,unit=0,drive=...
+
+and
+::
+
+-global isa-fdc.driveB=...
+-global sysbus-fdc.driveB=...
+
+become
+::
+
+-device floppy,unit=1,drive=...
+
+When plugging in a floppy controller
+::
+
+-device isa-fdc,...,driveA=...
+
+becomes
+::
+
+-device isa-fdc,...
+-device floppy,unit=0,drive=...
+
+and
+::
+
+-device isa-fdc,...,driveB=...
+
+becomes
+::
+
+-device isa-fdc,...
+-device floppy,unit=1,drive=...
+
QEMU Machine Protocol (QMP) commands
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 198940e737..f978ddf647 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -870,7 +870,6 @@ struct FDCtrl {
uint8_t num_floppies;
FDrive drives[MAX_FD];
struct {
-BlockBackend *blk;
FloppyDriveType type;
} qdev_for_drives[MAX_FD];
int reset_sensei;
@@ -2517,56 +2516,12 @@ static void fdctrl_connect_drives(FDCtrl *fdctrl,
DeviceState *fdc_dev,
{
unsigned int i;
FDrive *drive;
-DeviceState *dev;
-BlockBackend *blk;
-bool ok;
-const char *fdc_name, *drive_suffix;
for (i = 0; i < MAX_FD; i++) {
drive = &fdctrl->drives[i];
drive->fdctrl = fdctrl;
-
-/* If the drive is not present, we skip creating the qdev device, but
- * still have to initialise the controller. */
-blk = fdctrl->qdev_for_drives[i].blk;
-if (!blk) {
-fd_init(drive);
-fd_revalidate(drive);
-continue;
-}
-
-fdc_name = object_get_typename(OBJECT(fdc_dev));
-drive_suffix = !strcmp(fdc_name, "SUNW,fdtwo") ? "" : i ? "B" : "A";
-warn_report("warning: property %s.drive%s is deprecated",
-fdc_name, drive_suffix);
-error_printf("Use -device floppy,unit=%d,drive=... instead.\n", i);
-
-dev = qdev_new("floppy");
-qdev_prop_set_uint32(dev, "unit", i);
-qdev_prop_set_enum(dev, "drive-type", fdctrl->qdev_for_drives[i].type);
-
-/*
- * Hack alert: we move the backend from the floppy controller
- * device to the floppy device. We first need to detach the
- * controller, or else floppy_create()'s qdev_prop_set_drive()
- * will die when it attaches floppy device. We also need to
- * take another reference so that blk_detach_dev() doesn't
- * free blk while we still need it.
- *
- * The hack is probably a bad idea.
- */
-blk_ref(blk);
-blk_detach_dev(blk, fdc_dev);
-fdctrl->qd
[PATCH v4 3/4] fdc: Inline fdctrl_connect_drives() into fdctrl_realize_common()
The previous commit rendered the name fdctrl_connect_drives() somewhat
misleading. Get rid of it by inlining the (now pretty simple)
function into its only caller.
Signed-off-by: Markus Armbruster
Reviewed-by: Daniel P. Berrangé
---
hw/block/fdc.c | 23 ---
1 file changed, 8 insertions(+), 15 deletions(-)
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index f978ddf647..5210ab85e2 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -2511,20 +2511,6 @@ void isa_fdc_init_drives(ISADevice *fdc, DriveInfo **fds)
fdctrl_init_drives(&ISA_FDC(fdc)->state.bus, fds);
}
-static void fdctrl_connect_drives(FDCtrl *fdctrl, DeviceState *fdc_dev,
- Error **errp)
-{
-unsigned int i;
-FDrive *drive;
-
-for (i = 0; i < MAX_FD; i++) {
-drive = &fdctrl->drives[i];
-drive->fdctrl = fdctrl;
-fd_init(drive);
-fd_revalidate(drive);
-}
-}
-
void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
hwaddr mmio_base, DriveInfo **fds)
{
@@ -2604,7 +2590,14 @@ static void fdctrl_realize_common(DeviceState *dev,
FDCtrl *fdctrl,
}
floppy_bus_create(fdctrl, &fdctrl->bus, dev);
-fdctrl_connect_drives(fdctrl, dev, errp);
+
+for (i = 0; i < MAX_FD; i++) {
+FDrive *drive = &fdctrl->drives[i];
+
+drive->fdctrl = fdctrl;
+fd_init(drive);
+fd_revalidate(drive);
+}
}
static const MemoryRegionPortio fdc_portio_list[] = {
--
2.26.2
[PATCH v4 4/4] blockdev: Drop deprecated bogus -drive interface type
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(-)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 8d43b9336a..1279b57ee2 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -94,13 +94,6 @@ QEMU 5.1 has three options:
to the user to load all the images they need.
3. ``-bios `` - Tells QEMU to load the specified file as the firmwrae.
-``-drive`` with bogus interface type (since 5.1)
-
-
-Drives with interface types other than ``if=none`` are for onboard
-devices. It is possible to use drives the board doesn't pick up with
--device. This usage is now deprecated. Use ``if=none`` instead.
-
Short-form boolean options (since 6.0)
''
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index d10fc8cc17..40b1a5ede3 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -87,6 +87,13 @@ becomes
-device isa-fdc,...
-device floppy,unit=1,drive=...
+``-drive`` with bogus interface type (removed in 6.0)
+'
+
+Drives with interface types other than ``if=none`` are for onboard
+devices. Drives the board doesn't pick up can no longer be used with
+-device. Use ``if=none`` instead.
+
QEMU Machine Protocol (QMP) commands
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index 3b5fcda08d..32c2d6023c 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -35,7 +35,6 @@ struct DriveInfo {
bool is_default;/* Added by default_drive() ? */
int media_cd;
QemuOpts *opts;
-bool claimed_by_board;
QTAILQ_ENTRY(DriveInfo) next;
};
diff --git a/blockdev.c b/blockdev.c
index 68f022827c..c6149fe6a8 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -239,19 +239,10 @@ DriveInfo *drive_get(BlockInterfaceType type, int bus,
int unit)
return NULL;
}
-void drive_mark_claimed_by_board(void)
-{
-BlockBackend *blk;
-DriveInfo *dinfo;
-
-for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
-dinfo = blk_legacy_dinfo(blk);
-if (dinfo && blk_get_attached_dev(blk)) {
-dinfo->claimed_by_board = true;
-}
-}
-}
-
+/*
+ * Check board claimed all -drive that are meant to be claimed.
+ * Fatal error if any remain unclaimed.
+ */
void drive_check_orphaned(void)
{
BlockBackend *blk;
@@ -261,7 +252,17 @@ void drive_check_orphaned(void)
for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
dinfo = blk_legacy_dinfo(blk);
-if (dinfo->is_default || dinfo->type == IF_NONE) {
+/*
+ * Ignore default drives, because we create certain default
+ * drives unconditionally, then leave them unclaimed. Not the
+ * user's fault.
+ * Ignore IF_VIRTIO, because it gets desugared into -device.
+ * Leave failing to -device.
+ * Ignore IF_NONE, because leaving unclaimed IF_NONE available
+ * for device_add is a feature.
+ */
+if (dinfo->is_default || dinfo->type == IF_VIRTIO
+|| dinfo->type == IF_NONE) {
continue;
}
if (!blk_get_attached_dev(blk)) {
@@ -272,14 +273,6 @@ void drive_check_orphaned(void)
if_name[dinfo->type], dinfo->bus, dinfo->unit);
loc_pop(&loc);
orphans = true;
-continue;
-}
-if (!dinfo->claimed_by_board && dinfo->type != IF_VIRTIO) {
-loc_push_none(&loc);
-qemu_opts_loc_restore(dinfo->opts);
-warn_report("bogus if=%s is deprecated, use if=none",
-if_name[dinfo->type]);
-loc_pop(&loc);
}
}
diff --git a/softmmu/vl.c b/softmmu/vl.c
index ff488ea3e7..acaba95346 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2460,13 +2460,7 @@ static void qemu_init_board(void)
/* From here on we enter MACHINE_PHASE_INITIALIZED. */
machine_run_board_init(current_machine);
-/*
- * TODO To drop support for deprecated bogus if=..., move
- * drive_check_orphaned() here, replacing this call. Also drop
- * its deprecation warning, along with DriveInfo member
- * @claimed_by_board.
- */
-drive_mark_claimed_by_board();
+drive_check_orphaned();
realtime_in
[PATCH v4 1/4] docs/system/deprecated: Fix note on fdc drive properties
Commit 4a27a638e7 "fdc: Deprecate configuring floppies with -global isa-fdc" actually deprecated any use of floppy controller driver properties, not just with -global. Correct the deprecation note accordingly. Fixes: 4a27a638e718b445648de6b27c709353551d9b44 Signed-off-by: Markus Armbruster Reviewed-by: Daniel P. Berrangé --- docs/system/deprecated.rst | 33 - 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index 241b28a521..f272c3a414 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -94,10 +94,11 @@ QEMU 5.1 has three options: to the user to load all the images they need. 3. ``-bios `` - Tells QEMU to load the specified file as the firmwrae. -``Configuring floppies with ``-global`` -''' +Floppy controllers' drive properties (since 5.1) + -Use ``-device floppy,...`` instead: +Use ``-device floppy,...`` instead. When configuring onboard floppy +controllers :: -global isa-fdc.driveA=... @@ -120,8 +121,30 @@ become -device floppy,unit=1,drive=... -``-drive`` with bogus interface type - +When plugging in a floppy controller +:: + +-device isa-fdc,...,driveA=... + +becomes +:: + +-device isa-fdc,... +-device floppy,unit=0,drive=... + +and +:: + +-device isa-fdc,...,driveB=... + +becomes +:: + +-device isa-fdc,... +-device floppy,unit=1,drive=... + +``-drive`` with bogus interface type (since 5.1) + Drives with interface types other than ``if=none`` are for onboard devices. It is possible to use drives the board doesn't pick up with -- 2.26.2
Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
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 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. [1] https://gitlab.com/libvirt/libvirt/-/blob/master/tests/qemuxml2argvdata/numatune-memnode.args
Re: [PATCH v3 4/4] blockdev: Drop deprecated bogus -drive interface type
John Snow writes:
> On 3/9/21 11:12 AM, Markus Armbruster wrote:
>> Drop the crap deprecated in commit a1b40bda08 "blockdev: Deprecate
>> -drive with bogus interface type" (v5.1.0).
>>
>> 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 | 8 +--
>> 5 files changed, 23 insertions(+), 37 deletions(-)
>>
>> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
>> index 601e9647a5..664ed60e9f 100644
>> --- a/docs/system/deprecated.rst
>> +++ b/docs/system/deprecated.rst
>> @@ -94,13 +94,6 @@ QEMU 5.1 has three options:
>> to the user to load all the images they need.
>>3. ``-bios `` - Tells QEMU to load the specified file as the
>> firmwrae.
>>
>> -``-drive`` with bogus interface type (since 5.1)
>> -
>> -
>> -Drives with interface types other than ``if=none`` are for onboard
>> -devices. It is possible to use drives the board doesn't pick up with
>> --device. This usage is now deprecated. Use ``if=none`` instead.
>> -
>> Short-form boolean options (since 6.0)
>> ''
>>
>> diff --git a/docs/system/removed-features.rst
>> b/docs/system/removed-features.rst
>> index 77e7ba1339..e6d2fbe798 100644
>> --- a/docs/system/removed-features.rst
>> +++ b/docs/system/removed-features.rst
>> @@ -87,6 +87,13 @@ becomes
>> -device isa-fdc,...
>> -device floppy,unit=1,drive=...
>>
>> +``-drive`` with bogus interface type (removed in 6.0)
>> +'
>> +
>> +Drives with interface types other than ``if=none`` are for onboard
>> +devices. Drives the board doesn't pick up can no longer be used with
>> +-device. Use ``if=none`` instead.
>> +
>> QEMU Machine Protocol (QMP) commands
>>
>>
>> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
>> index 3b5fcda08d..32c2d6023c 100644
>> --- a/include/sysemu/blockdev.h
>> +++ b/include/sysemu/blockdev.h
>> @@ -35,7 +35,6 @@ struct DriveInfo {
>> bool is_default;/* Added by default_drive() ? */
>> int media_cd;
>> QemuOpts *opts;
>> -bool claimed_by_board;
>> QTAILQ_ENTRY(DriveInfo) next;
>> };
>>
>> diff --git a/blockdev.c b/blockdev.c
>> index cd438e60e3..2e01889cff 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -240,19 +240,10 @@ DriveInfo *drive_get(BlockInterfaceType type, int bus,
>> int unit)
>> return NULL;
>> }
>>
>> -void drive_mark_claimed_by_board(void)
>> -{
>> -BlockBackend *blk;
>> -DriveInfo *dinfo;
>> -
>> -for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
>> -dinfo = blk_legacy_dinfo(blk);
>> -if (dinfo && blk_get_attached_dev(blk)) {
>> -dinfo->claimed_by_board = true;
>> -}
>> -}
>> -}
>> -
>> +/*
>> + * Check board claimed all -drive that are meant to be claimed.
>> + * Fatal error if any remain unclaimed.
>> + */
>> void drive_check_orphaned(void)
>> {
>> BlockBackend *blk;
>> @@ -262,7 +253,17 @@ void drive_check_orphaned(void)
>>
>> for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
>> dinfo = blk_legacy_dinfo(blk);
>> -if (dinfo->is_default || dinfo->type == IF_NONE) {
>> +/*
>> + * Ignore default drives, because we create certain default
>> + * drives unconditionally, then leave them unclaimed. Not the
>> + * users fault.
>
> "user's" ?
Yes.
>> + * Ignore IF_VIRTIO, because it gets desugared into -device,
>> + * so we can leave failing to -device.
>> + * Ignore IF_NONE, because leaving unclaimed IF_NONE remains
>> + * available for device_add is a feature.
>
> Do you mean "as a feature" ?
I mean "because leaving unclaimed IF_NONE available for device_add is a
feature."
I'm embarrassingly prone to accidents when rewriting my own prose.
>> + */
>> +if (dinfo->is_default || dinfo->type == IF_VIRTIO
>> +|| dinfo->type == IF_NONE) {
>> continue;
>> }
>> if (!blk_get_attached_dev(blk)) {
>> @@ -273,14 +274,6 @@ void drive_check_orphaned(void)
>>if_name[dinfo->type], dinfo->bus, dinfo->unit);
>> loc_pop(&loc);
>> orphans = true;
>> -continue;
>> -}
>> -if (!dinfo->claimed_by_board && dinfo->type != IF_VIRTIO) {
>> -loc_push_none(&loc);
>> -qemu_opts_loc_restore(dinfo->opts);
>> -warn_report("bogus if=%s is deprecated, use if=none",
>> -if_name[dinfo->type]);
>> -loc_pop(&loc);
>> }
>> }
>>
>> diff --git a/soft
Re: [PATCH v3 4/4] blockdev: Drop deprecated bogus -drive interface type
On 3/9/21 11:12 AM, Markus Armbruster wrote:
Drop the crap deprecated in commit a1b40bda08 "blockdev: Deprecate
-drive with bogus interface type" (v5.1.0).
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 | 8 +--
5 files changed, 23 insertions(+), 37 deletions(-)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 601e9647a5..664ed60e9f 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -94,13 +94,6 @@ QEMU 5.1 has three options:
to the user to load all the images they need.
3. ``-bios `` - Tells QEMU to load the specified file as the firmwrae.
-``-drive`` with bogus interface type (since 5.1)
-
-
-Drives with interface types other than ``if=none`` are for onboard
-devices. It is possible to use drives the board doesn't pick up with
--device. This usage is now deprecated. Use ``if=none`` instead.
-
Short-form boolean options (since 6.0)
''
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index 77e7ba1339..e6d2fbe798 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -87,6 +87,13 @@ becomes
-device isa-fdc,...
-device floppy,unit=1,drive=...
+``-drive`` with bogus interface type (removed in 6.0)
+'
+
+Drives with interface types other than ``if=none`` are for onboard
+devices. Drives the board doesn't pick up can no longer be used with
+-device. Use ``if=none`` instead.
+
QEMU Machine Protocol (QMP) commands
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index 3b5fcda08d..32c2d6023c 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -35,7 +35,6 @@ struct DriveInfo {
bool is_default;/* Added by default_drive() ? */
int media_cd;
QemuOpts *opts;
-bool claimed_by_board;
QTAILQ_ENTRY(DriveInfo) next;
};
diff --git a/blockdev.c b/blockdev.c
index cd438e60e3..2e01889cff 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -240,19 +240,10 @@ DriveInfo *drive_get(BlockInterfaceType type, int bus,
int unit)
return NULL;
}
-void drive_mark_claimed_by_board(void)
-{
-BlockBackend *blk;
-DriveInfo *dinfo;
-
-for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
-dinfo = blk_legacy_dinfo(blk);
-if (dinfo && blk_get_attached_dev(blk)) {
-dinfo->claimed_by_board = true;
-}
-}
-}
-
+/*
+ * Check board claimed all -drive that are meant to be claimed.
+ * Fatal error if any remain unclaimed.
+ */
void drive_check_orphaned(void)
{
BlockBackend *blk;
@@ -262,7 +253,17 @@ void drive_check_orphaned(void)
for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
dinfo = blk_legacy_dinfo(blk);
-if (dinfo->is_default || dinfo->type == IF_NONE) {
+/*
+ * Ignore default drives, because we create certain default
+ * drives unconditionally, then leave them unclaimed. Not the
+ * users fault.
"user's" ?
+ * Ignore IF_VIRTIO, because it gets desugared into -device,
+ * so we can leave failing to -device.
+ * Ignore IF_NONE, because leaving unclaimed IF_NONE remains
+ * available for device_add is a feature.
Do you mean "as a feature" ?
+ */
+if (dinfo->is_default || dinfo->type == IF_VIRTIO
+|| dinfo->type == IF_NONE) {
continue;
}
if (!blk_get_attached_dev(blk)) {
@@ -273,14 +274,6 @@ void drive_check_orphaned(void)
if_name[dinfo->type], dinfo->bus, dinfo->unit);
loc_pop(&loc);
orphans = true;
-continue;
-}
-if (!dinfo->claimed_by_board && dinfo->type != IF_VIRTIO) {
-loc_push_none(&loc);
-qemu_opts_loc_restore(dinfo->opts);
-warn_report("bogus if=%s is deprecated, use if=none",
-if_name[dinfo->type]);
-loc_pop(&loc);
}
}
diff --git a/softmmu/vl.c b/softmmu/vl.c
index ff488ea3e7..7453611152 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2460,13 +2460,7 @@ static void qemu_init_board(void)
/* From here on we enter MACHINE_PHASE_INITIALIZED. */
machine_run_board_init(current_machine);
-/*
- * TODO To drop support for deprecated bogus if=..., move
- * drive_check_orphaned() here, replacing this call. Also drop
- * its deprecation warning, along with DriveInfo member
- * @claimed_by_board.
- */
-drive_mark_claimed_by_board();
+drive
[PATCH] vm : forbid to start a removing vm
From: Zhuang Shengen
When a vm is doing migration phase confirm, and then start it concurrently,
it will lead to the vm out of libvirtd control.
Cause Analysis:
1. thread1 migrate vm out.
2. thread2 start the migrating vm.
3. thread1 remove vm from domain list after migrate success.
4. thread2 acquired the vm job success and start the vm.
5. cannot find the vm any more by 'virsh list' command. Actually,
the started vm is not exist in the domain list.
Solution:
Check the vm->removing state before start.
Signed-off-by: Zhuang Shengen
Reviewed-by: Hogan Wang
---
src/qemu/qemu_driver.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d1a3659774..a5dfea94cb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6637,6 +6637,12 @@ qemuDomainCreateWithFlags(virDomainPtr dom, unsigned int
flags)
goto endjob;
}
+if (vm->removing) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("domain is already removing"));
+goto endjob;
+}
+
if (qemuDomainObjStart(dom->conn, driver, vm, flags,
QEMU_ASYNC_JOB_START) < 0)
goto endjob;
--
2.23.0
Re: [PATCH v2 04/12] qemu: capabilities: Introduce QEMU_CAPS_OBJECT_QAPIFIED
On a Wednesday in 2021, Peter Krempa wrote: Starting from qemu-6.0 the parameters of -object/object-add are formally described by the QAPI schema. Additionally this changes the nesting of the properties as the 'props' nested object will be flattened to the parent. We'll need to detect whether qemu switched to this new approach to generate the objects with proper nesting and also allow testing. The capability is based on the presence of the 'secret' object in the 'qom-type' enum. Signed-off-by: Peter Krempa --- src/qemu/qemu_capabilities.c | 4 src/qemu/qemu_capabilities.h | 3 +++ 2 files changed, 7 insertions(+) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH v2 03/12] qemu: command: Generate commandline of iothread objects JSON
On a Wednesday in 2021, Peter Krempa wrote: The commandline generator for 'iothread' objects has a private implementation of the properties. Convert it to JSON so that it can be later validated. Signed-off-by: Peter Krempa --- src/qemu/qemu_command.c | 18 +++--- 1 file changed, 11 insertions(+), 7 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH v2 02/12] qemu: command: Generate commandline of 'sev0' sev-guest object via JSON
On a Wednesday in 2021, Peter Krempa wrote: While the 'sev0' sev-guest object will never be hotplugged, but we want to generate it through JSON so that we'll be able to validate all parameters of '-object' against the QAPI schema once 'object-add' is qapified in qemu. Signed-off-by: Peter Krempa --- src/qemu/qemu_command.c | 32 +++ ...v-missing-platform-info.x86_64-2.12.0.args | 2 +- .../launch-security-sev.x86_64-2.12.0.args| 2 +- 3 files changed, 20 insertions(+), 16 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH v2 01/12] qemu: command: Generate commandline of 'masterKey0' secret via JSON
On a Wednesday in 2021, Peter Krempa wrote: While the 'masterKey0' secret object will never be hotplugged we want to generate it through JSON so that we'll be able to validate all parameters of '-object' against the QAPI schema once 'object-add' is qapified in qemu. Signed-off-by: Peter Krempa --- src/qemu/qemu_command.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH] XML validate that 'ramfb' has no address
On a Friday in 2021, Kristina Hanicova wrote: With this, XML fails if config video type 'ramfb' contains address, since address is not supported for 'ramfb' video devices. Previously it didn't raise error. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1891416 Signed-off-by: Kristina Hanicova --- src/conf/domain_validate.c | 8 1 file changed, 8 insertions(+) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH] virQEMUCapsInitQMPArch: Refactor cleanup
On a Wednesday in 2021, Yi Li wrote: Switch to using the 'g_auto*' helpers. Signed-off-by: Yi Li --- src/qemu/qemu_capabilities.c | 13 - 1 file changed, 4 insertions(+), 9 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 3/3] selinux: Remove 'make' dependency
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.
--
真実はいつも一つ！/ Always, there's only
Re: [PATCH 3/4] syntax-check: Update list of gethostname exceptions
On a Tuesday in 2021, Michal Privoznik wrote: The only place where gethostname() is acceptable is in virGetHostnameImpl() which lives in src/util/virutil.c. Reflect this in the list of exceptions for the syntax-check rule. Signed-off-by: Michal Privoznik --- build-aux/syntax-check.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 2/4] virutil: Do not use g_get_host_name() to obtain hostname
On a Tuesday in 2021, Michal Privoznik wrote: The problem is that g_get_host_name() caches the hostname in a thread local variable. Therefore, it doesn't reflect any subsequent hostname changes. While this might be acceptable for logs where the hostname is printed exactly once when the libvirtd starts up, it is not optimal for virGetHostnameImpl() which is what our public virConnectGetHostname() API calls. If the hostname at the moment of the first API invocation happens to be s/be/start with/ "localhost" or contains a dot, then no further hostname changes will ever be reflected. This reverts 26d9748ff11, partially. Signed-off-by: Michal Privoznik --- src/util/virutil.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 16/16] util: virstring: Remove virStrncpy
On a Tuesday in 2021, Peter Krempa wrote: The function is now unused and motivated users to write crazy parsers which were hard to understand, had pointless error paths just to avoid few memory allocations. Remove the function as we're fine with g_strndup and virStrcpy. Signed-off-by: Peter Krempa --- docs/coding-style.rst| 13 src/libvirt_private.syms | 1 - src/util/virstring.c | 44 src/util/virstring.h | 2 -- 4 files changed, 60 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 15/16] virNetLibsshAuthenticatePrivkeyCb: Use virStrcpy instead of virStrncpy
On a Tuesday in 2021, Peter Krempa wrote: We already assume that 'retr_passphrase.result' is a string, thus we can use virStrcpy instead. Signed-off-by: Peter Krempa --- src/rpc/virnetlibsshsession.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 14/16] virNetLibsshAuthenticatePrivkeyCb: Use g_autofree for 'actual_prompt'
On a Tuesday in 2021, Peter Krempa wrote: So that the 'error' label can be removed. Signed-off-by: Peter Krempa --- src/rpc/virnetlibsshsession.c | 14 -- 1 file changed, 4 insertions(+), 10 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 13/16] xenParseXLUSB: Rewrite to avoid virStrncpy
On a Tuesday in 2021, Peter Krempa wrote: Signed-off-by: Peter Krempa --- src/libxl/xen_xl.c | 24 ++-- 1 file changed, 6 insertions(+), 18 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 12/16] xenParseXLUSBController: Avoid use of virStrndup
On a Tuesday in 2021, Peter Krempa wrote: Use g_strndup with a freed buffer instead of the more complex approach using virStrndup. s/virStrndup/virStrncpy/ here and in the commit summary. Signed-off-by: Peter Krempa --- src/libxl/xen_xl.c | 51 ++ 1 file changed, 11 insertions(+), 40 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: RFC: do we want/need the "Ptr" typedefs for internal code ?
On Wed, Mar 10, 2021 at 06:41:35PM +0100, Michal Privoznik wrote: > On 3/10/21 4:36 PM, Michal Privoznik wrote: > > On 3/9/21 6:44 PM, Daniel P. Berrangé wrote: > > > One of the conventions we have had since the early days of libvirt is > > > that every struct typedef, has a corresponding "Ptr" typedef too. > > > > > > For example > > > > > > typedef struct _virDomainDef virDomainDef; > > > typedef virDomainDef *virDomainDefPtr; > > > > > > Periodically someone has questioned what the purpose of these Ptr > > > typedefs is, and we've not had an compelling answer, other than > > > that's what we've always done. > > > > > > > One possible upside is that it allows for less scary function headers, > > for instance: > > > > virDomainChrGetDomainPtrsInternal(virDomainDefPtr vmdef, > > virDomainChrDeviceType type, > > virDomainChrDefPtr ***arrPtr, > > size_t **cntPtr); > > > > Three levels of dereference don't look as bad as four. > > But yeah, I agree we should drop them. I wonder if cocci can help here. > > Or even plain in-place sed, except we'd have to be careful with > > statements like: > > > > virSomethingPtr a, b; > > > > where both @a and @b are pointers, and plain substitution would break it: > > > > virSomething *a, b; > > > > (here only @a is poitner, ofc). > > > > But since we have Peter nagging about this kind of variable declaration, > > it's not something one couldn't fix by hand. And in fact, whilst we're > > at it we could declare each variable at its own line. > > Okay, another problem: how does this g_auto() and similar work? I mean, now > we have: > > G_DEFINE_AUTO_CLEANUP_FREE_FUNC(virSysinfoDefPtr, virSysinfoDefFree, NULL); > > and it works fine. But obviously either of: > > G_DEFINE_AUTO_CLEANUP_FREE_FUNC(virSysinfoDef *, ...); > G_DEFINE_AUTO_CLEANUP_FREE_FUNC(virSysinfoDef, ...); > > is plain wrong. Maybe we can switch to g_autoptr() instead. Yes, we should be using g_autoptr even today, because g_auto is not for this use case: https://developer.gnome.org/glib/2.64/glib-Miscellaneous-Macros.html#g-auto "This is meant to be used with stack-allocated structures and non-pointer types. For the (more commonly used) pointer version, see g_autoptr()." 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 11/16] xenParseXLChannel: Use g_strndup instead of virStrndup
s/virStrndup/virStrncpy/ On a Wednesday in 2021, Ján Tomko wrote: On a Tuesday in 2021, Peter Krempa wrote: Make the temporary string a autofree-ing pointer and copy the contents. an Signed-off-by: Peter Krempa --- src/libxl/xen_xl.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 11/16] xenParseXLChannel: Use g_strndup instead of virStrndup
On a Tuesday in 2021, Peter Krempa wrote: Make the temporary string a autofree-ing pointer and copy the contents. an Signed-off-by: Peter Krempa --- src/libxl/xen_xl.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: RFC: do we want/need the "Ptr" typedefs for internal code ?
On Wed, Mar 10, 2021 at 18:41:35 +0100, Michal Privoznik wrote: > On 3/10/21 4:36 PM, Michal Privoznik wrote: > > On 3/9/21 6:44 PM, Daniel P. Berrangé wrote: > > > One of the conventions we have had since the early days of libvirt is > > > that every struct typedef, has a corresponding "Ptr" typedef too. > > > > > > For example > > > > > > typedef struct _virDomainDef virDomainDef; > > > typedef virDomainDef *virDomainDefPtr; > > > > > > Periodically someone has questioned what the purpose of these Ptr > > > typedefs is, and we've not had an compelling answer, other than > > > that's what we've always done. > > > > > > > One possible upside is that it allows for less scary function headers, > > for instance: > > > > virDomainChrGetDomainPtrsInternal(virDomainDefPtr vmdef, > > virDomainChrDeviceType type, > > virDomainChrDefPtr ***arrPtr, > > size_t **cntPtr); > > > > Three levels of dereference don't look as bad as four. > > But yeah, I agree we should drop them. I wonder if cocci can help here. > > Or even plain in-place sed, except we'd have to be careful with > > statements like: > > > > virSomethingPtr a, b; > > > > where both @a and @b are pointers, and plain substitution would break it: > > > > virSomething *a, b; > > > > (here only @a is poitner, ofc). > > > > But since we have Peter nagging about this kind of variable declaration, > > it's not something one couldn't fix by hand. And in fact, whilst we're > > at it we could declare each variable at its own line. Yup, we could possibly automatically fix the variable declarations to be one-per-line at the beginning. > > Okay, another problem: how does this g_auto() and similar work? I mean, now > we have: > > G_DEFINE_AUTO_CLEANUP_FREE_FUNC(virSysinfoDefPtr, virSysinfoDefFree, NULL); This is weird ... > > and it works fine. But obviously either of: > > G_DEFINE_AUTO_CLEANUP_FREE_FUNC(virSysinfoDef *, ...); > G_DEFINE_AUTO_CLEANUP_FREE_FUNC(virSysinfoDef, ...); > > is plain wrong. Maybe we can switch to g_autoptr() instead. .. so this is the right solution for that case.
Re: [PATCH 10/16] openvzReadNetworkConf: Rework parser
On a Tuesday in 2021, Peter Krempa wrote:
Rewrite so that the parser doesn't use virStrncpy by employing
g_strsplit.
Signed-off-by: Peter Krempa
---
src/openvz/openvz_conf.c | 77 ++--
1 file changed, 26 insertions(+), 51 deletions(-)
diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index 041a031a3a..836885af18 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -230,82 +230,57 @@ openvzReadNetworkConf(virDomainDefPtr def,
veid);
goto error;
} else if (ret > 0) {
-token = strtok_r(temp, ";", &saveptr);
-while (token != NULL) {
-char *p = token;
-char cpy_temp[32];
-int len;
+g_auto(GStrv) devices = g_strsplit(temp, ";", 0);
+GStrv device;
-/* add new device to list */
-net = g_new0(virDomainNetDef, 1);
+for (device = devices; device && *device; device++) {
+g_auto(GStrv) keyvals = g_strsplit(*device, ",", 0);
+GStrv keyval;
+net = g_new0(virDomainNetDef, 1);
net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
-/* parse string */
-do {
-char *next = strchr(p, ',');
-if (!next)
-next = strchr(p, '\0');
-if (STRPREFIX(p, "ifname=")) {
+for (keyval = keyvals; keyval && *keyval; keyval++) {
+char *val = strchr(*keyval, '=');
+
+if (!val)
+continue;
+
+val++;
+
+if (STRPREFIX(*keyval, "ifname=")) {
/* skip in libvirt */
-} else if (STRPREFIX(p, "host_ifname=")) {
-p += 12;
-len = next - p;
-if (len > 16) {
+} else if (STRPREFIX(*keyval, "host_ifname=")) {
+if (strlen(val) > 16) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Too long network device name"));
goto error;
}
-net->ifname = g_new0(char, len+1);
-
-if (virStrncpy(net->ifname, p, len, len+1) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Network ifname %s too long for
destination"), p);
-goto error;
-}
-} else if (STRPREFIX(p, "bridge=")) {
-p += 7;
-len = next - p;
-if (len > 16) {
+net->ifname = g_strdup(val);
+} else if (STRPREFIX(*keyval, "bridge=")) {
+if (strlen(val) > 16) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Too long bridge device name"));
goto error;
}
-net->data.bridge.brname = g_new0(char, len+1);
-
-if (virStrncpy(net->data.bridge.brname, p, len, len+1) <
0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Bridge name %s too long for
destination"), p);
-goto error;
-}
-} else if (STRPREFIX(p, "mac=")) {
-p += 4;
-len = next - p;
-if (len != 17) { /* should be 17 */
+net->data.bridge.brname = g_strdup(val);
+} else if (STRPREFIX(*keyval, "mac=")) {
+if (strlen(val) != 17) { /* should be 17 */
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Wrong length MAC address"));
goto error;
}
-if (virStrncpy(cpy_temp, p, len, sizeof(cpy_temp)) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("MAC address %s too long for
destination"), p);
-goto error;
-}
-if (virMacAddrParse(cpy_temp, &net->mac) < 0) {
+if (virMacAddrParse(val, &net->mac) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Wrong MAC address"));
goto error;
}
}
-p = ++next;
-} while (p < token + strlen(token));
-
-if (VIR_APPEND_ELEMENT_COPY(def->nets, def->nnets, net) < 0)
-goto error;
+}
-token = strtok_r(NULL, ";", &saveptr);
+ignore_value(VIR_APPEND_ELEMENT_COPY(def->nets, def->nnets, net));
Was this ignore_value needed because of some compiler warning?
Even
Re: RFC: do we want/need the "Ptr" typedefs for internal code ?
On 3/10/21 4:36 PM, Michal Privoznik wrote: On 3/9/21 6:44 PM, Daniel P. Berrangé wrote: One of the conventions we have had since the early days of libvirt is that every struct typedef, has a corresponding "Ptr" typedef too. For example typedef struct _virDomainDef virDomainDef; typedef virDomainDef *virDomainDefPtr; Periodically someone has questioned what the purpose of these Ptr typedefs is, and we've not had an compelling answer, other than that's what we've always done. One possible upside is that it allows for less scary function headers, for instance: virDomainChrGetDomainPtrsInternal(virDomainDefPtr vmdef, virDomainChrDeviceType type, virDomainChrDefPtr ***arrPtr, size_t **cntPtr); Three levels of dereference don't look as bad as four. But yeah, I agree we should drop them. I wonder if cocci can help here. Or even plain in-place sed, except we'd have to be careful with statements like: virSomethingPtr a, b; where both @a and @b are pointers, and plain substitution would break it: virSomething *a, b; (here only @a is poitner, ofc). But since we have Peter nagging about this kind of variable declaration, it's not something one couldn't fix by hand. And in fact, whilst we're at it we could declare each variable at its own line. Okay, another problem: how does this g_auto() and similar work? I mean, now we have: G_DEFINE_AUTO_CLEANUP_FREE_FUNC(virSysinfoDefPtr, virSysinfoDefFree, NULL); and it works fine. But obviously either of: G_DEFINE_AUTO_CLEANUP_FREE_FUNC(virSysinfoDef *, ...); G_DEFINE_AUTO_CLEANUP_FREE_FUNC(virSysinfoDef, ...); is plain wrong. Maybe we can switch to g_autoptr() instead. Michal
Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
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. 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
[PATCH v2] qemu: don't raise error upon interface update without for in coalesce
With this, incomplete XML without for in coalesce
won't raise error as before. It will leave the coalesce parameter
empty, thanks to passing it as a parameter and return an integer
to indicate error state - previously it returned pointer (or NULL
for both error and incomplete XML).
I also added a test case to test this functionality in the
qemuxml2xmltest.
The code went through some refactoring:
* change of a condition
* addition of a parameter
* change of order, that allowed removal of VIR_FREE
* removal of redundant labels and variables
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1535930
Signed-off-by: Kristina Hanicova
---
diff to v1:
- Added qemuxml2xmlout testcase, per Martin's review
src/conf/domain_conf.c| 25 +--
tests/qemuxml2argvdata/net-coalesce.xml | 9
tests/qemuxml2xmloutdata/net-coalesce.xml | 8 +++-
3 files changed, 26 insertions(+), 16 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b731744f04..798594f520 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7516,11 +7516,11 @@ virDomainNetIPInfoParseXML(const char *source,
}
-static virNetDevCoalescePtr
+static int
virDomainNetDefCoalesceParseXML(xmlNodePtr node,
-xmlXPathContextPtr ctxt)
+xmlXPathContextPtr ctxt,
+virNetDevCoalescePtr *coalesce)
{
-virNetDevCoalescePtr ret = NULL;
VIR_XPATH_NODE_AUTORESTORE(ctxt)
unsigned long long tmp = 0;
g_autofree char *str = NULL;
@@ -7529,15 +7529,13 @@ virDomainNetDefCoalesceParseXML(xmlNodePtr node,
str = virXPathString("string(./rx/frames/@max)", ctxt);
if (!str)
-return NULL;
-
-ret = g_new0(virNetDevCoalesce, 1);
+return 0;
if (virStrToLong_ullp(str, NULL, 10, &tmp) < 0) {
virReportError(VIR_ERR_XML_DETAIL,
_("cannot parse value '%s' for coalesce parameter"),
str);
-goto error;
+return -1;
}
if (tmp > UINT32_MAX) {
@@ -7545,15 +7543,13 @@ virDomainNetDefCoalesceParseXML(xmlNodePtr node,
_("value '%llu' is too big for coalesce "
"parameter, maximum is '%lu'"),
tmp, (unsigned long) UINT32_MAX);
-goto error;
+return -1;
}
-ret->rx_max_coalesced_frames = tmp;
-return ret;
+*coalesce = g_new0(virNetDevCoalesce, 1);
+(*coalesce)->rx_max_coalesced_frames = tmp;
- error:
-VIR_FREE(ret);
-return NULL;
+return 0;
}
static void
@@ -11517,8 +11513,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
node = virXPathNode("./coalesce", ctxt);
if (node) {
-def->coalesce = virDomainNetDefCoalesceParseXML(node, ctxt);
-if (!def->coalesce)
+if (virDomainNetDefCoalesceParseXML(node, ctxt, &def->coalesce) < 0)
goto error;
}
diff --git a/tests/qemuxml2argvdata/net-coalesce.xml
b/tests/qemuxml2argvdata/net-coalesce.xml
index bdcf786429..6fa3641e7d 100644
--- a/tests/qemuxml2argvdata/net-coalesce.xml
+++ b/tests/qemuxml2argvdata/net-coalesce.xml
@@ -55,6 +55,15 @@
+
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxml2xmloutdata/net-coalesce.xml
b/tests/qemuxml2xmloutdata/net-coalesce.xml
index 3a37587dde..452ed94de3 100644
--- a/tests/qemuxml2xmloutdata/net-coalesce.xml
+++ b/tests/qemuxml2xmloutdata/net-coalesce.xml
@@ -56,6 +56,12 @@
+
+
+
+
+
+
@@ -67,7 +73,7 @@
-
+
--
2.29.2
Re: [PATCH v2 0/2] XML non-virtio video device validation
Sorry everyone, it should have been v1. :) On Wed, Mar 10, 2021 at 5:43 PM Kristina Hanicova wrote: > > Kristina Hanicova (2): > move virDomainCheckVirtioOptionsAreAbsent a few lines forward > XML validate that non-virtio video devices have none virtio options > > src/conf/domain_validate.c | 60 -- > 1 file changed, 32 insertions(+), 28 deletions(-) > > -- > 2.29.2 > >
[PATCH v2 2/2] XML validate that non-virtio video devices have none virtio options
With this, XML fails if non-virtio video devices have virtio options. Previously it didn't raise error. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1922093 Signed-off-by: Kristina Hanicova --- src/conf/domain_validate.c | 4 1 file changed, 4 insertions(+) diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index dabdd7b8eb..b53f6437cc 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -179,6 +179,10 @@ virDomainVideoDefValidate(const virDomainVideoDef *video, } } +if (video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO && +(virDomainCheckVirtioOptionsAreAbsent(video->virtio) < 0)) +return -1; + return 0; } -- 2.29.2
[PATCH v2 0/2] XML non-virtio video device validation
Kristina Hanicova (2): move virDomainCheckVirtioOptionsAreAbsent a few lines forward XML validate that non-virtio video devices have none virtio options src/conf/domain_validate.c | 60 -- 1 file changed, 32 insertions(+), 28 deletions(-) -- 2.29.2
[PATCH v2 1/2] move virDomainCheckVirtioOptionsAreAbsent a few lines forward
Moving this function in order to use it in the next patch before
its previous declaration.
Signed-off-by: Kristina Hanicova
---
src/conf/domain_validate.c | 56 +++---
1 file changed, 28 insertions(+), 28 deletions(-)
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index b4e09e21fe..dabdd7b8eb 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -78,6 +78,34 @@ virDomainDefVideoValidate(const virDomainDef *def)
}
+static int
+virDomainCheckVirtioOptionsAreAbsent(virDomainVirtioOptionsPtr virtio)
+{
+if (!virtio)
+return 0;
+
+if (virtio->iommu != VIR_TRISTATE_SWITCH_ABSENT) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("iommu driver option is only supported "
+ "for virtio devices"));
+return -1;
+}
+if (virtio->ats != VIR_TRISTATE_SWITCH_ABSENT) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("ats driver option is only supported "
+ "for virtio devices"));
+return -1;
+}
+if (virtio->packed != VIR_TRISTATE_SWITCH_ABSENT) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("packed driver option is only supported "
+ "for virtio devices"));
+return -1;
+}
+return 0;
+}
+
+
static int
virDomainVideoDefValidate(const virDomainVideoDef *video,
const virDomainDef *def)
@@ -228,34 +256,6 @@
virSecurityDeviceLabelDefValidate(virSecurityDeviceLabelDefPtr *seclabels,
}
-static int
-virDomainCheckVirtioOptionsAreAbsent(virDomainVirtioOptionsPtr virtio)
-{
-if (!virtio)
-return 0;
-
-if (virtio->iommu != VIR_TRISTATE_SWITCH_ABSENT) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("iommu driver option is only supported "
- "for virtio devices"));
-return -1;
-}
-if (virtio->ats != VIR_TRISTATE_SWITCH_ABSENT) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("ats driver option is only supported "
- "for virtio devices"));
-return -1;
-}
-if (virtio->packed != VIR_TRISTATE_SWITCH_ABSENT) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("packed driver option is only supported "
- "for virtio devices"));
-return -1;
-}
-return 0;
-}
-
-
static int
virDomainDiskVhostUserValidate(const virDomainDiskDef *disk)
{
--
2.29.2
[PATCH 2/2] virtlo(g|ck)d: Fix exec-restart
Commit 94e45d1042e broke exec-restart of virtlogd and virtlockd as the
code waiting for the daemon shutdown closed the daemons before
exec-restarting.
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
--
2.29.2
[PATCH 1/2] virnetdaemon: Introduce virNetDaemonQuitExecRestart
Recent changes which meant to fix daemon shutdown broke the exec-restart
capability of virtlogd and virtlockd, since the code actually closed all
the sockets and shut down all the internals.
Add virNetDaemonQuitExecRestart, which requests a shutdown of the
process, but keeps all the services open and registered since they are
preserved across the restart.
Signed-off-by: Peter Krempa
---
src/libvirt_remote.syms | 1 +
src/rpc/virnetdaemon.c | 19 +++
src/rpc/virnetdaemon.h | 1 +
3 files changed, 21 insertions(+)
diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms
index 3cd84a0854..605271bcb2 100644
--- a/src/libvirt_remote.syms
+++ b/src/libvirt_remote.syms
@@ -85,6 +85,7 @@ virNetDaemonNew;
virNetDaemonNewPostExecRestart;
virNetDaemonPreExecRestart;
virNetDaemonQuit;
+virNetDaemonQuitExecRestart;
virNetDaemonRemoveShutdownInhibition;
virNetDaemonRun;
virNetDaemonSetShutdownCallbacks;
diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c
index 327540c4b4..cac60ef488 100644
--- a/src/rpc/virnetdaemon.c
+++ b/src/rpc/virnetdaemon.c
@@ -76,6 +76,7 @@ struct _virNetDaemon {
bool quit;
bool finished;
bool graceful;
+bool execRestart;
unsigned int autoShutdownTimeout;
size_t autoShutdownInhibitions;
@@ -857,6 +858,10 @@ virNetDaemonRun(virNetDaemonPtr dmn)
virHashForEach(dmn->servers, daemonServerProcessClients, NULL);
+/* don't shutdown services when performing an exec-restart */
+if (dmn->quit && dmn->execRestart)
+goto cleanup;
+
if (dmn->quit && dmn->finishTimer == -1) {
virHashForEach(dmn->servers, daemonServerClose, NULL);
if (dmn->shutdownPrepareCb && dmn->shutdownPrepareCb() < 0)
@@ -912,6 +917,20 @@ virNetDaemonQuit(virNetDaemonPtr dmn)
virObjectUnlock(dmn);
}
+
+void
+virNetDaemonQuitExecRestart(virNetDaemonPtr dmn)
+{
+virObjectLock(dmn);
+
+VIR_DEBUG("Exec-restart requested %p", dmn);
+dmn->quit = true;
+dmn->execRestart = true;
+
+virObjectUnlock(dmn);
+}
+
+
static int
daemonServerClose(void *payload,
const char *key G_GNUC_UNUSED,
diff --git a/src/rpc/virnetdaemon.h b/src/rpc/virnetdaemon.h
index fcc6e1fdff..b3d81b6aaf 100644
--- a/src/rpc/virnetdaemon.h
+++ b/src/rpc/virnetdaemon.h
@@ -75,6 +75,7 @@ void virNetDaemonSetStateStopWorkerThread(virNetDaemonPtr dmn,
void virNetDaemonRun(virNetDaemonPtr dmn);
void virNetDaemonQuit(virNetDaemonPtr dmn);
+void virNetDaemonQuitExecRestart(virNetDaemonPtr dmn);
bool virNetDaemonHasClients(virNetDaemonPtr dmn);
--
2.29.2
[PATCH 0/2] Fix exec-restart of virtlogd and virtlockd
Peter Krempa (2): virnetdaemon: Introduce virNetDaemonQuitExecRestart virtlo(g|ck)d: Fix exec-restart src/libvirt_remote.syms | 1 + src/locking/lock_daemon.c | 2 +- src/logging/log_daemon.c | 2 +- src/rpc/virnetdaemon.c| 19 +++ src/rpc/virnetdaemon.h| 1 + 5 files changed, 23 insertions(+), 2 deletions(-) -- 2.29.2
Re: [PATCH 09/16] xenParseSxprSound: Refactor parsing of model list
On a Tuesday in 2021, Peter Krempa wrote: Copy the input string so that we don't have to use a static buffer and virStrncpy. Signed-off-by: Peter Krempa --- src/libxl/xen_common.c | 46 +- 1 file changed, 19 insertions(+), 27 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: RFC: do we want/need the "Ptr" typedefs for internal code ?
On 3/9/21 6:44 PM, Daniel P. Berrangé wrote: One of the conventions we have had since the early days of libvirt is that every struct typedef, has a corresponding "Ptr" typedef too. For example typedef struct _virDomainDef virDomainDef; typedef virDomainDef *virDomainDefPtr; Periodically someone has questioned what the purpose of these Ptr typedefs is, and we've not had an compelling answer, other than that's what we've always done. One possible upside is that it allows for less scary function headers, for instance: virDomainChrGetDomainPtrsInternal(virDomainDefPtr vmdef, virDomainChrDeviceType type, virDomainChrDefPtr ***arrPtr, size_t **cntPtr); Three levels of dereference don't look as bad as four. But yeah, I agree we should drop them. I wonder if cocci can help here. Or even plain in-place sed, except we'd have to be careful with statements like: virSomethingPtr a, b; where both @a and @b are pointers, and plain substitution would break it: virSomething *a, b; (here only @a is poitner, ofc). But since we have Peter nagging about this kind of variable declaration, it's not something one couldn't fix by hand. And in fact, whilst we're at it we could declare each variable at its own line. Michal
Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
On Wed, Mar 10, 2021 at 15:31:57 +0100, Paolo Bonzini wrote: > 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". > > 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? I've got a slightly off-topic question/idea. Would it be reasonably easy/straightforward to run qemu's init code which parses arguments and possibly validates them but quit before actually starting to initiate resources? The use case would be to plug it (optionally?) into libvirt's qemuxml2argvtest to prevent such a thing from happening. It's not feasible to run all the configurations we have with a real VM but a partial validation would possibly make sense if it's easy enough.
Re: [PATCH 0/2] fix crash on invalid construction of a gvariant
On Wed, Mar 10, 2021 at 02:40:34PM +0100, Peter Krempa wrote: > Peter Krempa (2): > virSystemdCreateMachine: Use proper format string for uint64_t when > constructing gvariant > virsystemdtest: Call at least one virSystemdCreateMachine with > 'maxthreads' > 0 Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature
Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
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". 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?
Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
On Mon, Mar 08, 2021 at 17:54:10 +0100, Kevin Wolf wrote: > This series adds a QAPI type for the properties of all user creatable > QOM types and finally makes the --object command line option (in all > binaries) and the object-add monitor commands (in QMP and HMP) use the > new ObjectOptions union. > > This change improves things in more than just one way: > > 1. Documentation for QOM object types has always been lacking. Adding >the schema, we get documentation for every property. > > 2. It prevents bugs by performing parts of the input validation (e.g. >checking presence of mandatory properties) already in QAPI instead of >relying on separate manual implementations in each class. > > 3. It provides QAPI introspection for user creatable objects. > > 4. Non-scalar properties are now supported everywhere because the >command line parsers (including HMP) use the keyval parser now. > > > If you are in the CC list and didn't expect this series, it's probably > because you're the maintainer of one of the objects for which I'm adding > a QAPI schema description. Please just have a look at the specific patch > for your object and check whether the schema and its documentation make > sense to you. You can ignore all other patches. > > > In a next step after this series, we can add make use of the QAPI > structs in the implementation of the object and separate their > configuration from the runtime state. Specifically, the plan is to > add a .configure() callback to ObjectClass that allows configuring the > object in one place at creation time and keeping QOM property setters > only for properties that can actually be changed at runtime. Paolo made > an example of what the state could look like after this: > > https://wiki.qemu.org/Features/QOM-QAPI_integration > > Finally, the intention is to extend the QAPI schema to have separate > 'object' entities and generate some of the code that was written > manually in the intermediate state before. > > > This series is available as a git tag at: > > https://repo.or.cz/qemu/kevin.git qapi-object-v3 > > > v3: > - Removed now useless QAuthZListRuleListHack > - Made some more ObjectOptions branches conditional > - Improved documentation for some properties > - Fixed 'qemu-img compare' exit code for option parsing failure > > v2: > - Convert not only object-add, but all external interfaces so that the > schema will always be enforced and mismatch between implementation and > schema can't go unnoticed. > - Rebased, covering properties and object types added since v1 (yes, > things do become outdated rather quickly when you touch all user > creatable objects) > - Changed the "Since:" version number in the schema documentation to > refer to the version when the object was introduced rather than 6.0 > where the schema will (hopefully) be added > - Probably some other minor changes 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 Full commandline is: LC_ALL=C \ PATH=/root/.local/bin:/root/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin \ HOME=/var/lib/libvirt/qemu/domain-15-cd \ USER=root \ LOGNAME=root \ XDG_DATA_HOME=/var/lib/libvirt/qemu/domain-15-cd/.local/share \ XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain-15-cd/.cache \ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain-15-cd/.config \ /home/pipo/git/qemu.git/build/x86_64-softmmu/qemu-system-x86_64 \ -name guest=cd,debug-threads=on \ -S \ -object secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-15-cd/master-key.aes \ -machine pc-i440fx-2.9,accel=kvm,usb=off,vmport=off,dump-guest-core=off,memory-backend=pc.ram \ -cpu EPYC-Rome,x2apic=on,tsc-deadline=on,hypervisor=on,tsc-adjust=on,stibp=on,arch-capabilities=on,ssbd=on,xsaves=on,cmp-legacy=on,amd-ssbd=on,virt-ssbd=on,rdctl-no=on,skip-l1dfl-vmentry=on,mds-no=on,pschange-mc-no=on \ -m 1000 \ -object memory-backend-ram,id=pc.ram,size=1048576000,host-nodes=0,policy=bind \ -overcommit mem-lock=off \ -smp 2,maxcpus=8,sockets=8,cores=1,threads=1 \ -uuid 8e70100a-64b4-4186-aff9-e055c3075cb0 \ -no-user-config \ -nodefaults \ -device sga \ -chardev socket,id=charmonitor,fd=42,server=on,wait=off \ -mon chardev=charmonitor,id=monitor,mode=control \ -rtc base=utc,driftfix=slew \ -global kvm-pit.lost_tick_policy=delay \ -no-hpet \ -no-shutdown \ -global PIIX4_PM.disable_s3=1 \ -global PIIX4_PM.disable_s4=1 \ -boot menu=on,strict=on \ -device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x6.0x7 \ -device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x6 \ -device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x6.0x1 \ -device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x6.0x2 \ -device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x9 \
[PATCH 2/2] virsystemdtest: Call at least one virSystemdCreateMachine with 'maxthreads' > 0
There was a bug in the code adding TasksMax property. It remained
undetected because all tests used '0' for @maxthreads.
Signed-off-by: Peter Krempa
---
tests/virsystemdtest.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
index b48cdb950c..c082b95732 100644
--- a/tests/virsystemdtest.c
+++ b/tests/virsystemdtest.c
@@ -313,7 +313,7 @@ static int testCreateNetwork(const void *opaque
G_GNUC_UNUSED)
123,
true,
nnicindexes, nicindexes,
-"highpriority.slice", 0) < 0) {
+"highpriority.slice", 2) < 0) {
fprintf(stderr, "%s", "Failed to create LXC machine\n");
return -1;
}
--
2.29.2
[PATCH 0/2] fix crash on invalid construction of a gvariant
Peter Krempa (2): virSystemdCreateMachine: Use proper format string for uint64_t when constructing gvariant virsystemdtest: Call at least one virSystemdCreateMachine with 'maxthreads' > 0 src/util/virsystemd.c | 5 +++-- tests/virsystemdtest.c | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) -- 2.29.2
[PATCH 1/2] virSystemdCreateMachine: Use proper format string for uint64_t when constructing gvariant
g_variant_new_parsed uses '%t' for a uint64_t rather than printf-like
%llu. Additionally ensure that the passed value is a uint64_t since the
argument used is a 'unsigned int'.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1937287
Fixes: bf5f2ed09c2
Signed-off-by: Peter Krempa
---
src/util/virsystemd.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index c109324e96..c22f2df866 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -519,11 +519,12 @@ int virSystemdCreateMachine(const char *name,
}
if (maxthreads > 0) {
+uint64_t max = maxthreads;
+
if (!(scopename = virSystemdMakeScopeName(name, drivername, false)))
return -1;
-gprops = g_variant_new_parsed("[('TasksMax', <%llu>)]",
- (uint64_t)maxthreads);
+gprops = g_variant_new_parsed("[('TasksMax', <%t>)]", max);
message = g_variant_new("(sb@a(sv))",
scopename,
--
2.29.2
[PATCH 1/3] Add SELinux policy for virt
SELinux policy was created for:
Hypervisor drivers:
- virtqemud (QEMU/KVM)
- virtlxcd (LXC)
- virtvboxd (VirtualBox)
Secondary drivers:
- virtstoraged (host storage mgmt)
- virtnetworkd (virtual network mgmt)
- virtinterface (network interface mgmt)
- virtnodedevd (physical device mgmt)
- virtsecretd (security credential mgmt)
- virtnwfilterd (ip[6]tables/ebtables mgmt)
- virtproxyd (proxy daemon)
SELinux policy for virtvxz and virtxend has not been created yet, because I
wasn't able to reproduce AVC messages. These drivers run in unconfined_domain
until the AVC messages are reproduced internally and policy for these drivers
is made.
Signed-off-by: Nikola Knazekova
---
libvirt.spec.in | 62 ++
selinux/virt.fc | 111 +++
selinux/virt.if | 1984
selinux/virt.te | 2086 +++
4 files changed, 4243 insertions(+)
create mode 100644 selinux/virt.fc
create mode 100644 selinux/virt.if
create mode 100644 selinux/virt.te
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 8d8b900fbb..db08d91043 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -3,6 +3,13 @@
# This spec file assumes you are building on a Fedora or RHEL version
# that's still supported by the vendor. It may work on other distros
# or versions, but no effort will be made to ensure that going forward.
+
+%if 0%{?fedora} > 33 || 0%{?rhel} > 8
+ %global with_selinux 1
+ %global selinuxtype targeted
+ %global modulename virt
+%endif
+
%define min_rhel 7
%define min_fedora 31
@@ -256,6 +263,12 @@ Requires: libvirt-daemon-driver-nodedev =
%{version}-%{release}
Requires: libvirt-client = %{version}-%{release}
Requires: libvirt-libs = %{version}-%{release}
+%if 0%{?with_selinux}
+# This ensures that the *-selinux package and all it’s dependencies are not
pulled
+# into containers and other systems that do not use SELinux
+Requires: (%{name}-selinux if selinux-policy-%{selinuxtype})
+%endif
+
# All build-time requirements. Run-time requirements are
# listed against each sub-RPM
%if 0%{?rhel} == 7
@@ -983,6 +996,19 @@ Requires: libvirt-daemon-driver-network =
%{version}-%{release}
%description nss
Libvirt plugin for NSS for translating domain names into IP addresses.
+%if 0%{?with_selinux}
+# SELinux subpackage
+%package selinux
+Summary: Libvirt SELinux policy
+Requires: selinux-policy-%{selinuxtype}
+Requires(post): selinux-policy-%{selinuxtype}
+BuildRequires: selinux-policy-devel
+BuildArch: noarch
+%{?selinux_requires}
+
+%description selinux
+SELinux policy module for libvirt.
+%endif
%prep
@@ -1214,6 +1240,14 @@ 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}
@@ -1298,6 +1332,10 @@ 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
@@ -1506,6 +1544,24 @@ getent group virtlogin >/dev/null || groupadd -r
virtlogin
exit 0
%endif
+%if 0%{?with_selinux}
+# SELinux contexts are saved so that only affected files can be
+# relabeled after the policy module installation
+%pre selinux
+%selinux_relabel_pre -s %{selinuxtype}
+
+%post selinux
+%selinux_modules_install -s %{selinuxtype}
%{_datadir}/selinux/packages/%{selinuxtype}/%{modulename}.pp.bz2
+
+%postun selinux
+if [ $1 -eq 0 ]; then
+%selinux_modules_uninstall -s %{selinuxtype} %{modulename}
+fi
+
+%posttrans selinux
+%selinux_relabel_post -s %{selinuxtype}
+%endif
+
%files
%files docs
@@ -1972,5 +2028,11 @@ exit 0
%{_datadir}/libvirt/api/libvirt-qemu-api.xml
%{_datadir}/libvirt/api/libvirt-lxc-api.xml
+%if 0%{?with_selinux}
+%files selinux
+%{_datadir}/selinux/packages/%{selinuxtype}/%{modulename}.pp.*
+%ghost
%{_sharedstatedir}/selinux/%{selinuxtype}/active/modules/200/%{modulename}
+%endif
+
%changelog
diff --git a/selinux/virt.fc b/selinux/virt.fc
new file mode 100644
index 00..b7a2375ca1
--- /dev/null
+++ b/selinux/virt.fc
@@ -0,0 +1,111 @@
+HOME_DIR/\.libvirt(/.*)?
gen_context(system_u:object_r:virt_home_t,s0)
+HOME_DIR/\.libvirt/qemu(/.*)?
gen_context(system_u:object_r:svirt_home_t,s0)
+HOME_DIR/\.cache/libvirt(/.*)?
gen_context(system_u:object_r:virt_home_t,s0)
+HOME_DIR/\.cache/libvirt/qemu(/.*)?
gen_context(system_u:object_r:svirt_home_t,s0)
+HOME_DIR/\.config/libvirt(/.*)?
gen_context(system_u:object_r:virt_hom
[PATCH 3/3] selinux: Remove 'make' dependency
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
[PATCH 2/3] [DO NOT MERGE] Install selinux-policy-devel in test environment
From: Vit Mojzis Temporary commit for testing purposes. The change needs to be done in https://gitlab.com/libvirt/libvirt-ci/-/blob/master/guests/lcitool/lcitool/ansible/vars/projects/libvirt.yml Signed-off-by: Vit Mojzis --- ci/containers/ci-centos-8.Dockerfile | 1 + ci/containers/ci-centos-stream.Dockerfile| 1 + ci/containers/ci-fedora-32.Dockerfile| 1 + ci/containers/ci-fedora-33.Dockerfile| 1 + ci/containers/ci-fedora-rawhide-cross-mingw32.Dockerfile | 1 + ci/containers/ci-fedora-rawhide-cross-mingw64.Dockerfile | 1 + ci/containers/ci-fedora-rawhide.Dockerfile | 1 + 7 files changed, 7 insertions(+) diff --git a/ci/containers/ci-centos-8.Dockerfile b/ci/containers/ci-centos-8.Dockerfile index e600598329..7d6cbafe6b 100644 --- a/ci/containers/ci-centos-8.Dockerfile +++ b/ci/containers/ci-centos-8.Dockerfile @@ -84,6 +84,7 @@ RUN dnf update -y && \ rpm-build \ sanlock-devel \ scrub \ +selinux-policy-devel \ systemtap-sdt-devel \ wireshark-devel \ xfsprogs-devel \ diff --git a/ci/containers/ci-centos-stream.Dockerfile b/ci/containers/ci-centos-stream.Dockerfile index 2b51eccc8d..b4d02f4148 100644 --- a/ci/containers/ci-centos-stream.Dockerfile +++ b/ci/containers/ci-centos-stream.Dockerfile @@ -86,6 +86,7 @@ RUN dnf install -y centos-release-stream && \ rpm-build \ sanlock-devel \ scrub \ +selinux-policy-devel \ systemtap-sdt-devel \ wireshark-devel \ xfsprogs-devel \ diff --git a/ci/containers/ci-fedora-32.Dockerfile b/ci/containers/ci-fedora-32.Dockerfile index 71d391b7bd..3b9d98c83f 100644 --- a/ci/containers/ci-fedora-32.Dockerfile +++ b/ci/containers/ci-fedora-32.Dockerfile @@ -89,6 +89,7 @@ exec "$@"' > /usr/bin/nosync && \ rpm-build \ sanlock-devel \ scrub \ +selinux-policy-devel \ sheepdog \ systemtap-sdt-devel \ wireshark-devel \ diff --git a/ci/containers/ci-fedora-33.Dockerfile b/ci/containers/ci-fedora-33.Dockerfile index 5fb30380b0..c8b4dcca34 100644 --- a/ci/containers/ci-fedora-33.Dockerfile +++ b/ci/containers/ci-fedora-33.Dockerfile @@ -89,6 +89,7 @@ exec "$@"' > /usr/bin/nosync && \ rpm-build \ sanlock-devel \ scrub \ +selinux-policy-devel \ sheepdog \ systemtap-sdt-devel \ wireshark-devel \ diff --git a/ci/containers/ci-fedora-rawhide-cross-mingw32.Dockerfile b/ci/containers/ci-fedora-rawhide-cross-mingw32.Dockerfile index c718778acb..55825c9753 100644 --- a/ci/containers/ci-fedora-rawhide-cross-mingw32.Dockerfile +++ b/ci/containers/ci-fedora-rawhide-cross-mingw32.Dockerfile @@ -55,6 +55,7 @@ exec "$@"' > /usr/bin/nosync && \ rpcgen \ rpm-build \ scrub \ +selinux-policy-devel \ sheepdog \ zfs-fuse && \ nosync dnf autoremove -y && \ diff --git a/ci/containers/ci-fedora-rawhide-cross-mingw64.Dockerfile b/ci/containers/ci-fedora-rawhide-cross-mingw64.Dockerfile index 6058d0c0b2..69159a7e3c 100644 --- a/ci/containers/ci-fedora-rawhide-cross-mingw64.Dockerfile +++ b/ci/containers/ci-fedora-rawhide-cross-mingw64.Dockerfile @@ -55,6 +55,7 @@ exec "$@"' > /usr/bin/nosync && \ rpcgen \ rpm-build \ scrub \ +selinux-policy-devel \ sheepdog \ zfs-fuse && \ nosync dnf autoremove -y && \ diff --git a/ci/containers/ci-fedora-rawhide.Dockerfile b/ci/containers/ci-fedora-rawhide.Dockerfile index 027e8a7c41..edd9c34c46 100644 --- a/ci/containers/ci-fedora-rawhide.Dockerfile +++ b/ci/containers/ci-fedora-rawhide.Dockerfile @@ -90,6 +90,7 @@ exec "$@"' > /usr/bin/nosync && \ rpm-build \ sanlock-devel \ scrub \ +selinux-policy-devel \ sheepdog \ systemtap-sdt-devel \ wireshark-devel \ -- 2.29.2
Add SELinux policy for Virt
Hi, I created SELinux policy for Libvirt drivers, as part of Decentralized SELinux Policy (DSP) project. DSP guidelines is available: https://fedoraproject.org/wiki/SELinux/IndependentPolicy Discussion about the first version of SELinux policy for Libvirt is available on gitlab: https://gitlab.com/libvirt/libvirt/-/merge_requests/65 SELinux policy was created for: Hypervisor drivers: - virtqemud (QEMU/KVM) - virtlxcd (LXC) - virtvboxd (VirtualBox) Secondary drivers: - virtstoraged (host storage mgmt) - virtnetworkd (virtual network mgmt) - virtinterface (network interface mgmt) - virtnodedevd (physical device mgmt) - virtsecretd (security credential mgmt) - virtnwfilterd (ip[6]tables/ebtables mgmt) - virtproxyd (proxy daemon) SELinux policy for virtvxz and virtxend has not been created yet, because I wasn't able to reproduce AVC messages. These drivers run in unconfined_domain until the AVC messages are reproduced internally and policy for these drivers is made. Can you please look at it? Thanks Nikola
Libvirt has been accepted into GSoC 2021
Dear list, let me share great news: just like in the past years, libvirt is going to be part of Google Summer of Code also this year [1]. We can expect interested students to show up and discuss possible projects to work on. However, there are some changes made to this year's run [2]: 1) Smaller projects - with everything going on one can hardly expect students to commit to 350 hours of coding over three months. This is now reduced to 175 hours over 10 weeks, 2) More elligible students - now even PhD students and licensed coding school students can apply, 2) Halved stipends - since the amount of work is halved, so are the stipends, 3) Two evaluations - again, shorted time period requires less evaluations. Happy hacking, Michal 1: https://summerofcode.withgoogle.com/organizations/5771368592834560/ 2: https://opensource.googleblog.com/2020/10/google-summer-of-code-2021-is-bringing.html
Re: [PATCH v3 3/4] fdc: Inline fdctrl_connect_drives() into fdctrl_realize_common()
Richard Henderson writes:
> On 3/9/21 8:12 AM, Markus Armbruster wrote:
>> @@ -2565,6 +2551,7 @@ static void fdctrl_realize_common(DeviceState *dev,
>> FDCtrl *fdctrl,
>> Error **errp)
>> {
>> int i, j;
>> +FDrive *drive;
>> static int command_tables_inited = 0;
>> if (fdctrl->fallback == FLOPPY_DRIVE_TYPE_AUTO) {
>> @@ -2604,7 +2591,13 @@ static void fdctrl_realize_common(DeviceState *dev,
>> FDCtrl *fdctrl,
>> }
>> floppy_bus_create(fdctrl, &fdctrl->bus, dev);
>> -fdctrl_connect_drives(fdctrl, dev, errp);
>> +
>> +for (i = 0; i < MAX_FD; i++) {
>> +drive = &fdctrl->drives[i];
>
> FWIW, the declaration could be local to this loop.
Old-school habits. John, got a preference?
Re: [PATCH v3 4/4] blockdev: Drop deprecated bogus -drive interface type
Daniel P. Berrangé writes:
> On Tue, Mar 09, 2021 at 05:12:13PM +0100, Markus Armbruster wrote:
>> Drop the crap deprecated in commit a1b40bda08 "blockdev: Deprecate
>> -drive with bogus interface type" (v5.1.0).
>>
>> 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 | 8 +--
>> 5 files changed, 23 insertions(+), 37 deletions(-)
>>
>> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
>> index 601e9647a5..664ed60e9f 100644
>> --- a/docs/system/deprecated.rst
>> +++ b/docs/system/deprecated.rst
>> @@ -94,13 +94,6 @@ QEMU 5.1 has three options:
>>to the user to load all the images they need.
>> 3. ``-bios `` - Tells QEMU to load the specified file as the
>> firmwrae.
>>
>> -``-drive`` with bogus interface type (since 5.1)
>> -
>> -
>> -Drives with interface types other than ``if=none`` are for onboard
>> -devices. It is possible to use drives the board doesn't pick up with
>> --device. This usage is now deprecated. Use ``if=none`` instead.
>> -
>> Short-form boolean options (since 6.0)
>> ''
>>
>> diff --git a/docs/system/removed-features.rst
>> b/docs/system/removed-features.rst
>> index 77e7ba1339..e6d2fbe798 100644
>> --- a/docs/system/removed-features.rst
>> +++ b/docs/system/removed-features.rst
>> @@ -87,6 +87,13 @@ becomes
>> -device isa-fdc,...
>> -device floppy,unit=1,drive=...
>>
>> +``-drive`` with bogus interface type (removed in 6.0)
>> +'
>> +
>> +Drives with interface types other than ``if=none`` are for onboard
>> +devices. Drives the board doesn't pick up can no longer be used with
>> +-device. Use ``if=none`` instead.
>> +
>> QEMU Machine Protocol (QMP) commands
>>
>>
>> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
>> index 3b5fcda08d..32c2d6023c 100644
>> --- a/include/sysemu/blockdev.h
>> +++ b/include/sysemu/blockdev.h
>> @@ -35,7 +35,6 @@ struct DriveInfo {
>> bool is_default;/* Added by default_drive() ? */
>> int media_cd;
>> QemuOpts *opts;
>> -bool claimed_by_board;
>> QTAILQ_ENTRY(DriveInfo) next;
>> };
>>
>> diff --git a/blockdev.c b/blockdev.c
>> index cd438e60e3..2e01889cff 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -240,19 +240,10 @@ DriveInfo *drive_get(BlockInterfaceType type, int bus,
>> int unit)
>> return NULL;
>> }
>>
>> -void drive_mark_claimed_by_board(void)
>> -{
>> -BlockBackend *blk;
>> -DriveInfo *dinfo;
>> -
>> -for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
>> -dinfo = blk_legacy_dinfo(blk);
>> -if (dinfo && blk_get_attached_dev(blk)) {
>> -dinfo->claimed_by_board = true;
>> -}
>> -}
>> -}
>> -
>> +/*
>> + * Check board claimed all -drive that are meant to be claimed.
>> + * Fatal error if any remain unclaimed.
>> + */
>> void drive_check_orphaned(void)
>> {
>> BlockBackend *blk;
>> @@ -262,7 +253,17 @@ void drive_check_orphaned(void)
>>
>> for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
>> dinfo = blk_legacy_dinfo(blk);
>> -if (dinfo->is_default || dinfo->type == IF_NONE) {
>> +/*
>> + * Ignore default drives, because we create certain default
>> + * drives unconditionally, then leave them unclaimed. Not the
>> + * users fault.
>> + * Ignore IF_VIRTIO, because it gets desugared into -device,
>> + * so we can leave failing to -device.
>> + * Ignore IF_NONE, because leaving unclaimed IF_NONE remains
>> + * available for device_add is a feature.
>> + */
>> +if (dinfo->is_default || dinfo->type == IF_VIRTIO
>> +|| dinfo->type == IF_NONE) {
>> continue;
>> }
>> if (!blk_get_attached_dev(blk)) {
>> @@ -273,14 +274,6 @@ void drive_check_orphaned(void)
>> if_name[dinfo->type], dinfo->bus, dinfo->unit);
>> loc_pop(&loc);
>> orphans = true;
>> -continue;
>> -}
>> -if (!dinfo->claimed_by_board && dinfo->type != IF_VIRTIO) {
>> -loc_push_none(&loc);
>> -qemu_opts_loc_restore(dinfo->opts);
>> -warn_report("bogus if=%s is deprecated, use if=none",
>> -if_name[dinfo->type]);
>> -loc_pop(&loc);
>> }
>> }
>>
>> diff --git a/softmmu/vl.c b/softmmu/vl.c
>> index ff488ea3e7..7453611152 100644
>> --- a/softmmu/vl.c
>> +++ b/softmmu/vl.c
>> @@ -2460,13 +2460,7 @@ static void qemu_init_board(void)
>> /* From here on we enter MACHINE_PHASE_INIT
