[PATCH] Rework qemuMigrationDstPrecreateDisk()
'conn' vairable which are used only inside the func. Let's declare
inside the func body to make that obvious.
Use g_autofree to allow removal of 'cleanup:' and the 'ret' variable.
Signed-off-by: Yi Li
---
src/qemu/qemu_migration.c | 68 +++
1 file changed, 26 insertions(+), 42 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index f44d31c971..6bb0677f86 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -169,15 +169,15 @@ qemuMigrationSrcRestoreDomainState(virQEMUDriverPtr
driver, virDomainObjPtr vm)
static int
-qemuMigrationDstPrecreateDisk(virConnectPtr *conn,
- virDomainDiskDefPtr disk,
+qemuMigrationDstPrecreateDisk(virDomainDiskDefPtr disk,
unsigned long long capacity)
{
-int ret = -1;
-virStoragePoolPtr pool = NULL;
-virStorageVolPtr vol = NULL;
-char *volName = NULL, *basePath = NULL;
-char *volStr = NULL;
+g_autoptr(virConnect) conn = NULL;
+g_autoptr(virStoragePool) pool = NULL;
+g_autoptr(virStorageVol) vol = NULL;
+char *volName = NULL;
+g_autofree char *basePath = NULL;
+g_autofree char *volStr = NULL;
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
const char *format = NULL;
unsigned int flags = 0;
@@ -198,32 +198,28 @@ qemuMigrationDstPrecreateDisk(virConnectPtr *conn,
virReportError(VIR_ERR_INVALID_ARG,
_("malformed disk path: %s"),
disk->src->path);
-goto cleanup;
+return -1;
}
*volName = '\0';
volName++;
-if (!*conn) {
-if (!(*conn = virGetConnectStorage()))
-goto cleanup;
-}
+if (!(conn = virGetConnectStorage()))
+return -1;
-if (!(pool = virStoragePoolLookupByTargetPath(*conn, basePath)))
-goto cleanup;
+if (!(pool = virStoragePoolLookupByTargetPath(conn, basePath)))
+return -1;
format = virStorageFileFormatTypeToString(disk->src->format);
if (disk->src->format == VIR_STORAGE_FILE_QCOW2)
flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
break;
case VIR_STORAGE_TYPE_VOLUME:
-if (!*conn) {
-if (!(*conn = virGetConnectStorage()))
-goto cleanup;
-}
+if (!(conn = virGetConnectStorage()))
+return -1;
-if (!(pool = virStoragePoolLookupByName(*conn,
disk->src->srcpool->pool)))
-goto cleanup;
+if (!(pool = virStoragePoolLookupByName(conn,
disk->src->srcpool->pool)))
+return -1;
format = virStorageFileFormatTypeToString(disk->src->format);
volName = disk->src->srcpool->volume;
if (disk->src->format == VIR_STORAGE_FILE_QCOW2)
@@ -245,13 +241,13 @@ qemuMigrationDstPrecreateDisk(virConnectPtr *conn,
_("cannot precreate storage for disk type '%s'"),
virStorageTypeToString(disk->src->type));
goto cleanup;
+return -1;
}
if ((vol = virStorageVolLookupByName(pool, volName))) {
VIR_DEBUG("Skipping creation of already existing volume of name '%s'",
volName);
-ret = 0;
-goto cleanup;
+return 0;
}
virBufferAddLit(, "\n");
@@ -269,19 +265,13 @@ qemuMigrationDstPrecreateDisk(virConnectPtr *conn,
if (!(volStr = virBufferContentAndReset())) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("unable to create volume XML"));
-goto cleanup;
+return -1;
}
if (!(vol = virStorageVolCreateXML(pool, volStr, flags)))
-goto cleanup;
+return -1;
-ret = 0;
- cleanup:
-VIR_FREE(basePath);
-VIR_FREE(volStr);
-virObjectUnref(vol);
-virObjectUnref(pool);
-return ret;
+return 0;
}
static bool
@@ -313,9 +303,7 @@ qemuMigrationDstPrecreateStorage(virDomainObjPtr vm,
const char **migrate_disks,
bool incremental)
{
-int ret = -1;
size_t i = 0;
-virConnectPtr conn = NULL;
if (!nbd || !nbd->ndisks)
return 0;
@@ -332,7 +320,7 @@ qemuMigrationDstPrecreateStorage(virDomainObjPtr vm,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unable to find disk by target: %s"),
nbd->disks[i].target);
-goto cleanup;
+return -1;
}
if (disk->src->type == VIR_STORAGE_TYPE_NVME) {
@@ -352,20 +340,16 @@ qemuMigrationDstPrecreateStorage(virDomainObjPtr vm,
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("pre-creation of storage targets for incremental "
"storage migration is not supported"));
-
Re: [PATCH 2/4] hw/block/fdc: Remove the check_media_rate property
On 2/5/21 1:37 AM, Thomas Huth wrote:
On 05/02/2021 01.40, John Snow wrote:
On 2/3/21 12:18 PM, Thomas Huth wrote:
This was only required for the pc-1.0 and earlier machine types.
Now that these have been removed, we can also drop the corresponding
code from the FDC device.
Signed-off-by: Thomas Huth
---
hw/block/fdc.c | 17 ++---
tests/qemu-iotests/172.out | 35 ---
2 files changed, 2 insertions(+), 50 deletions(-)
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 292ea87805..198940e737 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -874,7 +874,6 @@ struct FDCtrl {
FloppyDriveType type;
} qdev_for_drives[MAX_FD];
int reset_sensei;
- uint32_t check_media_rate;
I am a bit of a dunce when it comes to the compatibility properties...
does this mess with the migration format?
I guess it doesn't, since it's not in the VMSTATE declaration.
H, alright.
I think that should be fine, yes.
FloppyDriveType fallback; /* type=auto failure fallback */
/* Timers state */
uint8_t timer0;
@@ -1021,18 +1020,10 @@ static const VMStateDescription
vmstate_fdrive_media_changed = {
}
};
-static bool fdrive_media_rate_needed(void *opaque)
-{
- FDrive *drive = opaque;
-
- return drive->fdctrl->check_media_rate;
-}
-
static const VMStateDescription vmstate_fdrive_media_rate = {
.name = "fdrive/media_rate",
.version_id = 1,
.minimum_version_id = 1,
- .needed = fdrive_media_rate_needed,
.fields = (VMStateField[]) {
VMSTATE_UINT8(media_rate, FDrive),
VMSTATE_END_OF_LIST()
@@ -1689,8 +1680,7 @@ static void fdctrl_start_transfer(FDCtrl
*fdctrl, int direction)
/* Check the data rate. If the programmed data rate does not match
* the currently inserted medium, the operation has to fail. */
- if (fdctrl->check_media_rate &&
- (fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
+ if ((fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
FLOPPY_DPRINTF("data rate mismatch (fdc=%d, media=%d)\n",
fdctrl->dsr & FD_DSR_DRATEMASK,
cur_drv->media_rate);
fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_MA, 0x00);
@@ -2489,8 +2479,7 @@ static void fdctrl_result_timer(void *opaque)
cur_drv->sect = (cur_drv->sect % cur_drv->last_sect) + 1;
}
/* READ_ID can't automatically succeed! */
- if (fdctrl->check_media_rate &&
- (fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
+ if ((fdctrl->dsr & FD_DSR_DRATEMASK) != cur_drv->media_rate) {
FLOPPY_DPRINTF("read id rate mismatch (fdc=%d, media=%d)\n",
fdctrl->dsr & FD_DSR_DRATEMASK,
cur_drv->media_rate);
fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_MA, 0x00);
@@ -2895,8 +2884,6 @@ static Property isa_fdc_properties[] = {
DEFINE_PROP_UINT32("dma", FDCtrlISABus, dma, 2),
DEFINE_PROP_DRIVE("driveA", FDCtrlISABus,
state.qdev_for_drives[0].blk),
DEFINE_PROP_DRIVE("driveB", FDCtrlISABus,
state.qdev_for_drives[1].blk),
- DEFINE_PROP_BIT("check_media_rate", FDCtrlISABus,
state.check_media_rate,
- 0, true),
Could you theoretically set this via QOM commands in QMP, and claim
that this is a break in behavior?
Though, it's ENTIRELY undocumented, so ... it's probably fine, I
think. Probably. (Please soothe my troubled mind.)
A user actually could mess with this property even on the command line,
e.g. by using:
qemu-system-x86_64 -global isa-fdc.check_media_rate=false
... but, as you said, it's completely undocumented, the property is
really just there for the internal use of machine type compatibility.
We've done such clean-ups in the past already, see e.g.
c6026998eef382d7ad76 or 2a4dbaf1c0db2453ab78f, so I think this should be
fine. But if you disagree, I could replace this by a patch that adds
this property to the list of deprecated features instead, so we could at
least remove it after it has been deprecated for two releases?
I don't think it's necessary, personally -- just wanted to make sure I
knew the exact stakes here.
Reviewed-by: John Snow
Acked-by: John Snow
Re: [PATCH] bhyve: relax emulator binary value check
Michal Privoznik wrote:
> On 2/4/21 4:47 PM, Roman Bogorodskiy wrote:
> > Currently, requesting domain capabilities fails when the specified
> > emulator binary does not equal to "/usr/sbin/bhyve". Relax this check
> > to allow any path with basename "bhyve", as it's a common case when
> > binary is specified without an absolute path.
> >
> > Signed-off-by: Roman Bogorodskiy
> > ---
> > I'm not sure how useful is this check in general: at this point we don't
> > use the user specified emulator value anyway, just use BHYVE constant.
> > Probably it would be better to just drop the entire "else" block?
>
> Yes, I was just about to suggest that. We don't do any check like this
> for QEMU driver. Just execute user provided binary (with sume arguments
> appended to get QMP monitor) and query caps.
>
> Looking into bhyve driver code though, it doesn't use from
> domain XML, does it? What I'm getting at is that there is no way for a
> user to specify different binary to run than /usr/sbin/bhyve. So it
> doesn't really make sense to provide emulatorbin at all.
Yes, currently we just use BHYVE (from config.h) to build commands.
In general, I've just realized that it's a little bit messy right now.
Even if we switch command line builder to use user specified value,
this will not work properly because we still use virFindFileInPath("bhyve");
to populate driver->bhyvecaps.
I guess the right way would be to move most of the code from
virBhyveProbeCaps() to virBhyveDomainCapsFill() and try to use the user
specified binary.
> Since I can argue both ways, merge this patch or just remove the block
> completely.
I'll just remove the block then.
Thanks,
> Reviewed-by: Michal Privoznik
>
> Michal
>
Roman Bogorodskiy
signature.asc
Description: PGP signature
[PULL v2 08/16] hw/i386: Remove the deprecated pc-1.x machine types
From: Thomas Huth
They have been deprecated since QEMU v5.0, time to remove them now.
Signed-off-by: Thomas Huth
Message-Id: <20210203171832.483176-2-th...@redhat.com>
Reviewed-by: Michael S. Tsirkin
Signed-off-by: Michael S. Tsirkin
Reviewed-by: Daniel P. Berrangé
---
hw/i386/pc_piix.c| 94
docs/system/deprecated.rst | 6 --
docs/system/removed-features.rst | 6 ++
3 files changed, 6 insertions(+), 100 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 6188c3e97e..2904b40163 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -359,18 +359,6 @@ static void pc_compat_1_4_fn(MachineState *machine)
pc_compat_1_5_fn(machine);
}
-static void pc_compat_1_3(MachineState *machine)
-{
-pc_compat_1_4_fn(machine);
-}
-
-/* PC compat function for pc-1.0 to pc-1.2 */
-static void pc_compat_1_2(MachineState *machine)
-{
-pc_compat_1_3(machine);
-x86_cpu_change_kvm_default("kvm-pv-eoi", NULL);
-}
-
static void pc_init_isa(MachineState *machine)
{
pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, TYPE_I440FX_PCI_DEVICE);
@@ -772,88 +760,6 @@ static void pc_i440fx_1_4_machine_options(MachineClass *m)
DEFINE_I440FX_MACHINE(v1_4, "pc-i440fx-1.4", pc_compat_1_4_fn,
pc_i440fx_1_4_machine_options);
-static void pc_i440fx_1_3_machine_options(MachineClass *m)
-{
-X86MachineClass *x86mc = X86_MACHINE_CLASS(m);
-static GlobalProperty compat[] = {
-PC_CPU_MODEL_IDS("1.3.0")
-{ "usb-tablet", "usb_version", "1" },
-{ "virtio-net-pci", "ctrl_mac_addr", "off" },
-{ "virtio-net-pci", "mq", "off" },
-{ "e1000", "autonegotiation", "off" },
-};
-
-pc_i440fx_1_4_machine_options(m);
-m->hw_version = "1.3.0";
-m->deprecation_reason = "use a newer machine type instead";
-x86mc->compat_apic_id_mode = true;
-compat_props_add(m->compat_props, compat, G_N_ELEMENTS(compat));
-}
-
-DEFINE_I440FX_MACHINE(v1_3, "pc-1.3", pc_compat_1_3,
- pc_i440fx_1_3_machine_options);
-
-
-static void pc_i440fx_1_2_machine_options(MachineClass *m)
-{
-static GlobalProperty compat[] = {
-PC_CPU_MODEL_IDS("1.2.0")
-{ "nec-usb-xhci", "msi", "off" },
-{ "nec-usb-xhci", "msix", "off" },
-{ "qxl", "revision", "3" },
-{ "qxl-vga", "revision", "3" },
-{ "VGA", "mmio", "off" },
-};
-
-pc_i440fx_1_3_machine_options(m);
-m->hw_version = "1.2.0";
-compat_props_add(m->compat_props, compat, G_N_ELEMENTS(compat));
-}
-
-DEFINE_I440FX_MACHINE(v1_2, "pc-1.2", pc_compat_1_2,
- pc_i440fx_1_2_machine_options);
-
-
-static void pc_i440fx_1_1_machine_options(MachineClass *m)
-{
-static GlobalProperty compat[] = {
-PC_CPU_MODEL_IDS("1.1.0")
-{ "virtio-scsi-pci", "hotplug", "off" },
-{ "virtio-scsi-pci", "param_change", "off" },
-{ "VGA", "vgamem_mb", "8" },
-{ "vmware-svga", "vgamem_mb", "8" },
-{ "qxl-vga", "vgamem_mb", "8" },
-{ "qxl", "vgamem_mb", "8" },
-{ "virtio-blk-pci", "config-wce", "off" },
-};
-
-pc_i440fx_1_2_machine_options(m);
-m->hw_version = "1.1.0";
-compat_props_add(m->compat_props, compat, G_N_ELEMENTS(compat));
-}
-
-DEFINE_I440FX_MACHINE(v1_1, "pc-1.1", pc_compat_1_2,
- pc_i440fx_1_1_machine_options);
-
-static void pc_i440fx_1_0_machine_options(MachineClass *m)
-{
-static GlobalProperty compat[] = {
-PC_CPU_MODEL_IDS("1.0")
-{ TYPE_ISA_FDC, "check_media_rate", "off" },
-{ "virtio-balloon-pci", "class", stringify(PCI_CLASS_MEMORY_RAM) },
-{ "apic-common", "vapic", "off" },
-{ TYPE_USB_DEVICE, "full-path", "no" },
-};
-
-pc_i440fx_1_1_machine_options(m);
-m->hw_version = "1.0";
-compat_props_add(m->compat_props, compat, G_N_ELEMENTS(compat));
-}
-
-DEFINE_I440FX_MACHINE(v1_0, "pc-1.0", pc_compat_1_2,
- pc_i440fx_1_0_machine_options);
-
-
typedef struct {
uint16_t gpu_device_id;
uint16_t pch_device_id;
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 6ac757ed9f..2fcac7861e 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -322,12 +322,6 @@ The 'scsi-disk' device is deprecated. Users should use
'scsi-hd' or
System emulator machines
-``pc-1.0``, ``pc-1.1``, ``pc-1.2`` and ``pc-1.3`` (since 5.0)
-'
-
-These machine types are very old and likely can not be used for live migration
-from old QEMU versions anymore. A newer machine type should be used instead.
-
Raspberry Pi ``raspi2`` and ``raspi3`` machines (since 5.2)
'''
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index 88b81a6156..c8481cafbd 100644
---
Re: [PATCH] docs: Add 'known_hosts_verify' parameter for libssh(2) connection uris
On 2/5/21 2:53 PM, Jakob Meng wrote: Signed-off-by: Jakob Meng Thanks for the review! Feel free to drop or change the URLs, e.g. to permalinks: https://gitlab.com/libvirt/libvirt/-/blob/f209d40a7e74e7e53a02c0c7ed20be218d390754/src/rpc/virnetsocket.c#L941 https://gitlab.com/libvirt/libvirt/-/blob/f209d40a7e74e7e53a02c0c7ed20be218d390754/src/rpc/virnetsocket.c#L1073 I've dropped them. Reviewed-by: Michal Privoznik and pushed. Congratulations on your first libvirt contribution! Michal
Re: [libvirt PATCH 6/6] tools: report messages for 'dominfo' command
On Friday, 5 February 2021 15:18:31 CET Daniel P. Berrangé wrote:
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index 02ff1fbd62..df24543ef8 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -1291,6 +1291,7 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd)
> char *str, uuid[VIR_UUID_STRING_BUFLEN];
> int has_managed_save = 0;
> virshControlPtr priv = ctl->privData;
> +char **messages = NULL;
>
> if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
> return false;
> @@ -1391,6 +1392,18 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd)
> VIR_FREE(seclabel);
> }
> }
> +
> +if (virDomainGetMessages(dom, , 0) > 0) {
> +size_t i;
> +for (i = 0; messages[i] != NULL; i++) {
> +if (i == 0) {
> +vshPrint(ctl, "%-15s %s\n", _("Messages:"), messages[i]);
> +} else {
> +vshPrint(ctl, "%-15s %s\n", "", messages[i]);
> +}
> +}
> +}
'messages' is leaked here.
--
Pino Toscano
signature.asc
Description: This is a digitally signed message part.
Re: [libvirt PATCH 5/6] qemu: implement virDomainGetMessages API
On Friday, 5 February 2021 15:18:30 CET Daniel P. Berrangé wrote:
> +for (i = 0; i < VIR_DOMAIN_TAINT_LAST; i++) {
> +if (vm->taint & (1 << i)) {
> +(*msgs)[n++] = g_strdup_printf(
> +"%s: %s", _("tainted"),
Please translate the whole string, instead of composing a string
puzzle, e.g.: g_strdup_printf(_("tainted: %s"), msg)
There are languages that may need to tweak capitalization and/or
punctuation of text; for example, in French a space is needed
before and after : (colon).
> +if (!flags || (flags & VIR_DOMAIN_MESSAGE_DEPRECATION)) {
> +nmsgs += vm->ndeprecations;
> +*msgs = g_renew(char *, *msgs, nmsgs+1);
> +
> +for (i = 0; i < vm->ndeprecations; i++) {
> +(*msgs)[n++] = g_strdup_printf(
> +"%s: %s",
> +_("deprecated configuration"),
> +vm->deprecations[i]);
Ditto.
--
Pino Toscano
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH 2/3] utils: Deprecate hex-with-suffix sizes
04.02.2021 22:07, Eric Blake wrote:
Supporting '0x20M' looks odd, particularly since we have an 'E' suffix
What about also deprecating 'E' suffix? (just my problem of reviewing previous
patch)
that is ambiguous between a hex digit and the extremely large exibyte
suffix, as well as a 'B' suffix for bytes. In practice, people using
hex inputs are specifying values in bytes (and would have written
0x200, or possibly relied on default_suffix in the case of
qemu_strtosz_MiB), and the use of scaling suffixes makes the most
sense for inputs in decimal (where the user would write 32M). But
rather than outright dropping support for hex-with-suffix, let's
follow our deprecation policy. Sadly, since qemu_strtosz() does not
have an Err** parameter, we pollute to stderr.
Signed-off-by: Eric Blake
---
docs/system/deprecated.rst | 8
util/cutils.c | 6 +-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 6ac757ed9fa7..c404c3926e6f 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -146,6 +146,14 @@ library enabled as a cryptography provider.
Neither the ``nettle`` library, or the built-in cryptography provider are
supported on FIPS enabled hosts.
+hexadecimal sizes with scaling multipliers (since 6.0)
+''
+
+Input parameters that take a size value should only use a size suffix
+(such as 'k' or 'M') when the base is written in decimal, and not when
+the value is hexadecimal. That is, '0x20M' is deprecated, and should
+be written either as '32M' or as '0x200'.
+
QEMU Machine Protocol (QMP) commands
diff --git a/util/cutils.c b/util/cutils.c
index 0234763bd70b..75190565cbb5 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -264,7 +264,7 @@ static int do_strtosz(const char *nptr, const char **end,
int retval;
const char *endptr;
unsigned char c;
-bool mul_required = false;
+bool mul_required = false, hex = false;
uint64_t val;
int64_t mul;
double fraction = 0.0;
@@ -309,6 +309,10 @@ static int do_strtosz(const char *nptr, const char **end,
you forget to set hex to true in corresponding if(){...}
c = *endptr;
mul = suffix_mul(c, unit);
if (mul > 0) {
+if (hex) {
+fprintf(stderr, "Using a multiplier suffix on hex numbers "
+"is deprecated: %s\n", nptr);
+}
endptr++;
} else {
mul = suffix_mul(default_suffix, unit);
should we also deprecate hex where default_suffix is not 'B' ?
--
Best regards,
Vladimir
[libvirt PATCH 5/6] qemu: implement virDomainGetMessages API
Signed-off-by: Daniel P. Berrangé
---
src/conf/domain_conf.c | 17
src/conf/domain_conf.h | 1 +
src/libvirt_private.syms | 2 ++
src/qemu/qemu_domain.h | 3 ++
src/qemu/qemu_driver.c | 59
5 files changed, 82 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a873c0ada2..f78fc992c1 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -90,6 +90,23 @@ VIR_ENUM_IMPL(virDomainTaint,
"deprecated-config",
);
+VIR_ENUM_IMPL(virDomainTaintMessage,
+ VIR_DOMAIN_TAINT_LAST,
+ N_("custom configuration parameters specified"),
+ N_("custom monitor control commands issued"),
+ N_("running with undesirable elevated privileges"),
+ N_("network configuration using opaque shell scripts"),
+ N_("potentially unsafe disk format probing"),
+ N_("managing externally launched configuration"),
+ N_("potentially unsafe use of host CPU passthrough"),
+ N_("configuration potentially modified by hook script"),
+ N_("use of host cdrom passthrough"),
+ N_("custom device tree blob used"),
+ N_("custom guest agent control commands issued"),
+ N_("hypervisor feature autodetection override"),
+ N_("use of deprecated configuration settings"),
+);
+
VIR_ENUM_IMPL(virDomainVirt,
VIR_DOMAIN_VIRT_LAST,
"none",
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ea6370c03d..1ef4266d13 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3631,6 +3631,7 @@ bool virDomainVsockDefEquals(const virDomainVsockDef *a,
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT;
VIR_ENUM_DECL(virDomainTaint);
+VIR_ENUM_DECL(virDomainTaintMessage);
VIR_ENUM_DECL(virDomainVirt);
VIR_ENUM_DECL(virDomainBoot);
VIR_ENUM_DECL(virDomainFeature);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 512da526fc..730289a1f8 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -628,6 +628,8 @@ virDomainStateTypeToString;
virDomainStorageNetworkParseHost;
virDomainStorageSourceParse;
virDomainStorageSourceParseBase;
+virDomainTaintMessageTypeFromString;
+virDomainTaintMessageTypeToString;
virDomainTaintTypeFromString;
virDomainTaintTypeToString;
virDomainTimerModeTypeFromString;
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 7453881a31..42b6fda91a 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -572,6 +572,9 @@ void qemuDomainObjTaintMsg(virQEMUDriverPtr driver,
const char *msg,
...) G_GNUC_PRINTF(5, 6);
+char **qemuDomainObjGetTainting(virQEMUDriverPtr driver,
+virDomainObjPtr obj);
+
void qemuDomainObjCheckTaint(virQEMUDriverPtr driver,
virDomainObjPtr obj,
qemuDomainLogContextPtr logCtxt,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c34af6b7d1..fee4cb81ed 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20367,6 +20367,64 @@ qemuDomainAuthorizedSSHKeysSet(virDomainPtr dom,
}
+static int
+qemuDomainGetMessages(virDomainPtr dom,
+ char ***msgs,
+ unsigned int flags)
+{
+virDomainObjPtr vm = NULL;
+int rv = -1;
+size_t i, n;
+int nmsgs;
+
+virCheckFlags(VIR_DOMAIN_MESSAGE_DEPRECATION |
+ VIR_DOMAIN_MESSAGE_TAINTING, -1);
+
+if (!(vm = qemuDomainObjFromDomain(dom)))
+return -1;
+
+if (virDomainGetMessagesEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+*msgs = NULL;
+nmsgs = 0;
+n = 0;
+
+if (!flags || (flags & VIR_DOMAIN_MESSAGE_TAINTING)) {
+nmsgs += __builtin_popcount(vm->taint);
+*msgs = g_renew(char *, *msgs, nmsgs+1);
+
+for (i = 0; i < VIR_DOMAIN_TAINT_LAST; i++) {
+if (vm->taint & (1 << i)) {
+(*msgs)[n++] = g_strdup_printf(
+"%s: %s", _("tainted"),
+_(virDomainTaintMessageTypeToString(i)));
+}
+}
+}
+
+if (!flags || (flags & VIR_DOMAIN_MESSAGE_DEPRECATION)) {
+nmsgs += vm->ndeprecations;
+*msgs = g_renew(char *, *msgs, nmsgs+1);
+
+for (i = 0; i < vm->ndeprecations; i++) {
+(*msgs)[n++] = g_strdup_printf(
+"%s: %s",
+_("deprecated configuration"),
+vm->deprecations[i]);
+}
+}
+
+(*msgs)[nmsgs] = NULL;
+
+rv = nmsgs;
+
+ cleanup:
+virDomainObjEndAPI();
+return rv;
+}
+
+
static virHypervisorDriver qemuHypervisorDriver = {
.name = QEMU_DRIVER_NAME,
.connectURIProbe = qemuConnectURIProbe,
@@ -20608,6 +20666,7 @@ static
[libvirt PATCH 6/6] tools: report messages for 'dominfo' command
$ virsh dominfo demo
Id: 2
Name: demo
UUID: eadf8ef0-bf14-4c5f-9708-4a19bacf9e81
OS Type:hvm
State: running
CPU(s): 2
CPU time: 15.8s
Max memory: 1536000 KiB
Used memory:1536000 KiB
Persistent: yes
Autostart: disable
Managed save: no
Security model: selinux
Security DOI: 0
Security label: unconfined_u:unconfined_r:svirt_t:s0:c443,c956 (permissive)
Messages: tainted: custom monitor control commands issued
tainted: use of deprecated configuration settings
deprecated configuration: machine type 'pc-1.2'
Signed-off-by: Daniel P. Berrangé
---
tools/virsh-domain-monitor.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 02ff1fbd62..df24543ef8 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1291,6 +1291,7 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd)
char *str, uuid[VIR_UUID_STRING_BUFLEN];
int has_managed_save = 0;
virshControlPtr priv = ctl->privData;
+char **messages = NULL;
if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
return false;
@@ -1391,6 +1392,18 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd)
VIR_FREE(seclabel);
}
}
+
+if (virDomainGetMessages(dom, , 0) > 0) {
+size_t i;
+for (i = 0; messages[i] != NULL; i++) {
+if (i == 0) {
+vshPrint(ctl, "%-15s %s\n", _("Messages:"), messages[i]);
+} else {
+vshPrint(ctl, "%-15s %s\n", "", messages[i]);
+}
+}
+}
+
virshDomainFree(dom);
return ret;
}
--
2.29.2
[libvirt PATCH 3/6] src: define virDomainGetMessages API
This API allows fetching a list of informational messages recorded
against the domain. This provides a way to give information about
tainting of the guest due to undesirable actions/configs, as well
as provide details of deprecated features.
The output of this API is explicitly targetted at humans, not
machines, so it is inappropriate to attempt to pattern match on
the strings and take action off them, not least because the messages
are marked for translation.
Should there be a demand for machine targetted information, this
would have to be addressed via a new API, and is not planned at
this point in time.
Signed-off-by: Daniel P. Berrangé
---
include/libvirt/libvirt-domain.h | 9 ++
src/driver-hypervisor.h | 6
src/libvirt-domain.c | 54
src/libvirt_public.syms | 5 +++
4 files changed, 74 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index de2456812c..8011cf9fe1 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -5119,4 +5119,13 @@ int virDomainAuthorizedSSHKeysSet(virDomainPtr domain,
unsigned int nkeys,
unsigned int flags);
+typedef enum {
+VIR_DOMAIN_MESSAGE_DEPRECATION = (1 << 0),
+VIR_DOMAIN_MESSAGE_TAINTING = (1 << 1),
+} virDomainMessageType;
+
+int virDomainGetMessages(virDomainPtr domain,
+ char ***msgs,
+ unsigned int flags);
+
#endif /* LIBVIRT_DOMAIN_H */
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index 9e8fe89921..05d7dfb5c7 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -1400,6 +1400,11 @@ typedef int
unsigned int nkeys,
unsigned int flags);
+typedef int
+(*virDrvDomainGetMessages)(virDomainPtr domain,
+ char ***msgs,
+ unsigned int flags);
+
typedef struct _virHypervisorDriver virHypervisorDriver;
typedef virHypervisorDriver *virHypervisorDriverPtr;
@@ -1665,4 +1670,5 @@ struct _virHypervisorDriver {
virDrvDomainBackupGetXMLDesc domainBackupGetXMLDesc;
virDrvDomainAuthorizedSSHKeysGet domainAuthorizedSSHKeysGet;
virDrvDomainAuthorizedSSHKeysSet domainAuthorizedSSHKeysSet;
+virDrvDomainGetMessages domainGetMessages;
};
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index dba89a7d3a..ae318f4a1a 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -13102,3 +13102,57 @@ virDomainAuthorizedSSHKeysSet(virDomainPtr domain,
virDispatchError(conn);
return -1;
}
+
+
+/**
+ * virDomainGetMessages:
+ * @domain: a domain object
+ * @msgs: pointer to a variable to store messages
+ * @flags: zero or more virDomainMessageType flags
+ *
+ * Fetch a list of all messages recorded against the VM and
+ * store them into @msgs array which is allocated upon
+ * successful return and is NULL terminated. The caller is
+ * responsible for freeing @msgs when no longer needed.
+ *
+ * If @flags is zero then all messages are reported. The
+ * virDomainMessageType constants can be used to restrict
+ * results to certain types of message.
+ *
+ * Note it is hypervisor dependant whether messages are
+ * available for shutoff guests, or running guests, or
+ * both. Thus a client should be prepared to re-fetch
+ * messages when a guest transitions between running
+ * and shutoff states.
+ *
+ * Returns: number of messages stored in @msgs,
+ * -1 otherwise.
+ */
+int
+virDomainGetMessages(virDomainPtr domain,
+ char ***msgs,
+ unsigned int flags)
+{
+virConnectPtr conn;
+
+VIR_DOMAIN_DEBUG(domain, "msgs=%p, flags=0x%x", msgs, flags);
+
+virResetLastError();
+
+virCheckDomainReturn(domain, -1);
+conn = domain->conn;
+virCheckNonNullArgGoto(msgs, error);
+
+if (conn->driver->domainGetMessages) {
+int ret;
+ret = conn->driver->domainGetMessages(domain, msgs, flags);
+if (ret < 0)
+goto error;
+return ret;
+}
+
+virReportUnsupportedError();
+ error:
+virDispatchError(conn);
+return -1;
+}
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index cf31f937d5..d851333eb0 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -879,4 +879,9 @@ LIBVIRT_6.10.0 {
virDomainAuthorizedSSHKeysSet;
} LIBVIRT_6.0.0;
+LIBVIRT_7.1.0 {
+global:
+virDomainGetMessages;
+} LIBVIRT_6.10.0;
+
# define new API here using predicted next version number
--
2.29.2
[libvirt PATCH 1/6] conf: record deprecation messages against the domain
These messages will be stored in the live status XML.
Signed-off-by: Daniel P. Berrangé
---
src/conf/domain_conf.c | 28 +---
src/conf/domain_conf.h | 4
src/libvirt_private.syms | 1 +
3 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 07e6f39256..a873c0ada2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1788,6 +1788,15 @@ bool virDomainObjTaint(virDomainObjPtr obj,
}
+void virDomainObjDeprecation(virDomainObjPtr obj,
+ const char *msg)
+{
+obj->deprecations = g_renew(char *, obj->deprecations,
+obj->ndeprecations + 1);
+obj->deprecations[obj->ndeprecations++] = g_strdup(msg);
+}
+
+
static void
virDomainGraphicsAuthDefClear(virDomainGraphicsAuthDefPtr def)
{
@@ -21225,7 +21234,8 @@ virDomainObjParseXML(xmlDocPtr xml,
int reason = 0;
void *parseOpaque = NULL;
g_autofree char *tmp = NULL;
-g_autofree xmlNodePtr *nodes = NULL;
+g_autofree xmlNodePtr *taintNodes = NULL;
+g_autofree xmlNodePtr *depNodes = NULL;
if (!(obj = virDomainObjNew(xmlopt)))
return NULL;
@@ -21272,10 +21282,10 @@ virDomainObjParseXML(xmlDocPtr xml,
}
obj->pid = (pid_t)val;
-if ((n = virXPathNodeSet("./taint", ctxt, )) < 0)
+if ((n = virXPathNodeSet("./taint", ctxt, )) < 0)
goto error;
for (i = 0; i < n; i++) {
-char *str = virXMLPropString(nodes[i], "flag");
+char *str = virXMLPropString(taintNodes[i], "flag");
if (str) {
int flag = virDomainTaintTypeFromString(str);
if (flag < 0) {
@@ -21289,6 +21299,13 @@ virDomainObjParseXML(xmlDocPtr xml,
}
}
+if ((n = virXPathNodeSet("./deprecation", ctxt, )) < 0)
+goto error;
+for (i = 0; i < n; i++) {
+g_autofree char *str = virXMLNodeContentString(depNodes[i]);
+virDomainObjDeprecation(obj, str);
+}
+
if (xmlopt->privateData.parse &&
xmlopt->privateData.parse(ctxt, obj, >config) < 0)
goto error;
@@ -29122,6 +29139,11 @@ virDomainObjFormat(virDomainObjPtr obj,
virDomainTaintTypeToString(i));
}
+for (i = 0; i < obj->ndeprecations; i++) {
+virBufferEscapeString(, "%s\n",
+ obj->deprecations[i]);
+}
+
if (xmlopt->privateData.format &&
xmlopt->privateData.format(, obj) < 0)
return NULL;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 8b1c8643be..ea6370c03d 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2801,6 +2801,8 @@ struct _virDomainObj {
void (*privateDataFreeFunc)(void *);
int taint;
+size_t ndeprecations;
+char **deprecations;
unsigned long long original_memlock; /* Original RLIMIT_MEMLOCK, zero if no
* restore will be required later */
@@ -3058,6 +3060,8 @@ void virDomainObjEndAPI(virDomainObjPtr *vm);
bool virDomainObjTaint(virDomainObjPtr obj,
virDomainTaintFlags taint);
+void virDomainObjDeprecation(virDomainObjPtr obj,
+ const char *msg);
void virDomainObjBroadcast(virDomainObjPtr vm);
int virDomainObjWait(virDomainObjPtr vm);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0636b0d8c9..512da526fc 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -547,6 +547,7 @@ virDomainObjAssignDef;
virDomainObjBroadcast;
virDomainObjCheckActive;
virDomainObjCopyPersistentDef;
+virDomainObjDeprecation;
virDomainObjEndAPI;
virDomainObjFormat;
virDomainObjGetDefs;
--
2.29.2
[libvirt PATCH 4/6] remote: add RPC support for the virDomainGetMessages API
Signed-off-by: Daniel P. Berrangé
---
src/remote/remote_daemon_dispatch.c | 45 +
src/remote/remote_driver.c | 44
src/remote/remote_protocol.x| 21 +-
src/remote_protocol-structs | 11 +++
4 files changed, 120 insertions(+), 1 deletion(-)
diff --git a/src/remote/remote_daemon_dispatch.c
b/src/remote/remote_daemon_dispatch.c
index b7ef1f4b26..e9f2a0ce5b 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -7463,3 +7463,48 @@ remoteDispatchDomainAuthorizedSshKeysSet(virNetServerPtr
server G_GNUC_UNUSED,
return rv;
}
+
+static int
+remoteDispatchDomainGetMessages(virNetServerPtr server G_GNUC_UNUSED,
+virNetServerClientPtr client,
+virNetMessagePtr msg G_GNUC_UNUSED,
+virNetMessageErrorPtr rerr,
+remote_domain_get_messages_args *args,
+remote_domain_get_messages_ret *ret)
+{
+int rv = -1;
+virConnectPtr conn = remoteGetHypervisorConn(client);
+int nmsgs = 0;
+char **msgs = NULL;
+virDomainPtr dom = NULL;
+
+if (!conn)
+goto cleanup;
+
+if (!(dom = get_nonnull_domain(conn, args->dom)))
+goto cleanup;
+
+if ((nmsgs = virDomainGetMessages(dom, , args->flags)) < 0)
+goto cleanup;
+
+if (nmsgs > REMOTE_DOMAIN_MESSAGES_MAX) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Number of msgs %d, which exceeds max limit: %d"),
+ nmsgs, REMOTE_DOMAIN_MESSAGES_MAX);
+goto cleanup;
+}
+
+ret->msgs.msgs_val = g_steal_pointer();
+ret->msgs.msgs_len = nmsgs;
+
+rv = nmsgs;
+
+ cleanup:
+if (rv < 0)
+virNetMessageSaveError(rerr);
+if (nmsgs > 0)
+virStringListFreeCount(msgs, nmsgs);
+virObjectUnref(dom);
+
+return rv;
+}
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 8d6790ccf2..a83cd866e7 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -8098,6 +8098,49 @@ remoteDomainAuthorizedSSHKeysSet(virDomainPtr domain,
}
+static int
+remoteDomainGetMessages(virDomainPtr domain,
+char ***msgs,
+unsigned int flags)
+{
+int rv = -1;
+size_t i;
+struct private_data *priv = domain->conn->privateData;
+remote_domain_get_messages_args args;
+remote_domain_get_messages_ret ret;
+
+remoteDriverLock(priv);
+
+make_nonnull_domain(, domain);
+args.flags = flags;
+memset(, 0, sizeof(ret));
+
+if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_MESSAGES,
+ (xdrproc_t) xdr_remote_domain_get_messages_args, (char *),
+ (xdrproc_t) xdr_remote_domain_get_messages_ret, (char *)) ==
-1) {
+goto cleanup;
+}
+
+if (ret.msgs.msgs_len > REMOTE_DOMAIN_MESSAGES_MAX) {
+virReportError(VIR_ERR_RPC, "%s",
+ _("remoteDomainGetMessages: "
+ "returned number of msgs exceeds limit"));
+goto cleanup;
+}
+
+*msgs = g_new0(char *, ret.msgs.msgs_len + 1);
+for (i = 0; i < ret.msgs.msgs_len; i++)
+(*msgs)[i] = g_strdup(ret.msgs.msgs_val[i]);
+
+rv = ret.msgs.msgs_len;
+
+ cleanup:
+remoteDriverUnlock(priv);
+xdr_free((xdrproc_t)xdr_remote_domain_get_messages_ret,
+ (char *) );
+return rv;
+}
+
/* get_nonnull_domain and get_nonnull_network turn an on-wire
* (name, uuid) pair into virDomainPtr or virNetworkPtr object.
* These can return NULL if underlying memory allocations fail,
@@ -8531,6 +8574,7 @@ static virHypervisorDriver hypervisor_driver = {
.domainBackupGetXMLDesc = remoteDomainBackupGetXMLDesc, /* 6.0.0 */
.domainAuthorizedSSHKeysGet = remoteDomainAuthorizedSSHKeysGet, /* 6.10.0
*/
.domainAuthorizedSSHKeysSet = remoteDomainAuthorizedSSHKeysSet, /* 6.10.0
*/
+.domainGetMessages = remoteDomainGetMessages, /* 7.1.0 */
};
static virNetworkDriver network_driver = {
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 2df38cef77..d3724bc305 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -283,6 +283,9 @@ const REMOTE_NETWORK_PORT_PARAMETERS_MAX = 16;
/* Upper limit on number of SSH keys */
const REMOTE_DOMAIN_AUTHORIZED_SSH_KEYS_MAX = 2048;
+/* Upper limit on number of messages */
+const REMOTE_DOMAIN_MESSAGES_MAX = 2048;
+
/* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */
typedef opaque remote_uuid[VIR_UUID_BUFLEN];
@@ -3799,6 +3802,16 @@ struct remote_domain_authorized_ssh_keys_set_args {
unsigned int flags;
};
+struct remote_domain_get_messages_args {
+remote_nonnull_domain dom;
+unsigned int flags;
+};
+
+struct remote_domain_get_messages_ret {
+remote_nonnull_string msgs;
+};
[libvirt PATCH 2/6] qemu: record deprecation messages against the domain
These messages are only valid while the domain is running.
Signed-off-by: Daniel P. Berrangé
---
src/qemu/qemu_domain.c | 5 +
src/qemu/qemu_process.c | 5 +
2 files changed, 10 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0f09e321fb..d362764060 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6236,6 +6236,11 @@ void qemuDomainObjTaintMsg(virQEMUDriverPtr driver,
va_end(args);
}
+if (taint == VIR_DOMAIN_TAINT_DEPRECATED_CONFIG &&
+extramsg) {
+virDomainObjDeprecation(obj, extramsg);
+}
+
VIR_WARN("Domain id=%d name='%s' uuid=%s is tainted: %s%s%s%s",
obj->def->id,
obj->def->name,
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 30cfa4d485..91f74b95bb 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7857,6 +7857,11 @@ void qemuProcessStop(virQEMUDriverPtr driver,
}
}
+for (i = 0; i < vm->ndeprecations; i++)
+g_free(vm->deprecations[i]);
+g_free(vm->deprecations);
+vm->ndeprecations = 0;
+vm->deprecations = NULL;
vm->taint = 0;
vm->pid = -1;
virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);
--
2.29.2
[libvirt PATCH 0/6] APIs for reporting tainting and deprecation messages
This is a follow up to https://listman.redhat.com/archives/libvir-list/2021-January/msg00988.html I pushed the first non-API parts of that series already. This posting takes a different approach to the APIs. Instead of separte APIs for tainting and deprecations, there is now one API for reporting general informational messages. This is explicitly only targetted at humans. Daniel P. Berrangé (6): conf: record deprecation messages against the domain qemu: record deprecation messages against the domain src: define virDomainGetMessages API remote: add RPC support for the virDomainGetMessages API qemu: implement virDomainGetMessages API tools: report messages for 'dominfo' command include/libvirt/libvirt-domain.h| 9 + src/conf/domain_conf.c | 45 -- src/conf/domain_conf.h | 5 +++ src/driver-hypervisor.h | 6 +++ src/libvirt-domain.c| 54 ++ src/libvirt_private.syms| 3 ++ src/libvirt_public.syms | 5 +++ src/qemu/qemu_domain.c | 5 +++ src/qemu/qemu_domain.h | 3 ++ src/qemu/qemu_driver.c | 59 + src/qemu/qemu_process.c | 5 +++ src/remote/remote_daemon_dispatch.c | 45 ++ src/remote/remote_driver.c | 44 + src/remote/remote_protocol.x| 21 +- src/remote_protocol-structs | 11 ++ tools/virsh-domain-monitor.c| 13 +++ 16 files changed, 329 insertions(+), 4 deletions(-) -- 2.29.2
Re: [PATCH 2/3] utils: Deprecate hex-with-suffix sizes
On Fri, Feb 05, 2021 at 07:40:36AM -0600, Eric Blake wrote:
> On 2/5/21 5:13 AM, Daniel P. Berrangé wrote:
> > On Thu, Feb 04, 2021 at 01:07:07PM -0600, Eric Blake wrote:
> >> Supporting '0x20M' looks odd, particularly since we have an 'E' suffix
> >> that is ambiguous between a hex digit and the extremely large exibyte
> >> suffix, as well as a 'B' suffix for bytes. In practice, people using
> >> hex inputs are specifying values in bytes (and would have written
> >> 0x200, or possibly relied on default_suffix in the case of
> >> qemu_strtosz_MiB), and the use of scaling suffixes makes the most
> >> sense for inputs in decimal (where the user would write 32M). But
> >> rather than outright dropping support for hex-with-suffix, let's
> >> follow our deprecation policy. Sadly, since qemu_strtosz() does not
> >> have an Err** parameter, we pollute to stderr.
> >
> > Err** is only appropriate for errors, not warnings, so this statement
> > can be removed.
>
> The point of the comment was that we have no other mechanism for
> reporting the deprecation other than polluting to stderr. And if we
> ever remove the support, we'll either have to silently reject input that
> we used to accept, or plumb through Err** handling to allow better
> reporting of WHY we are rejecting input. But I didn't feel like adding
> Err** support now.
Yeah, I guess what I meant was that warning on stderr is the expected
standard practice for deprecations. We only need to worry about other
thing once the deprecation turns into a hard error later.
>
> >> @@ -309,6 +309,10 @@ static int do_strtosz(const char *nptr, const char
> >> **end,
> >> c = *endptr;
> >> mul = suffix_mul(c, unit);
> >> if (mul > 0) {
> >> +if (hex) {
> >> +fprintf(stderr, "Using a multiplier suffix on hex numbers "
> >> +"is deprecated: %s\n", nptr);
> >
> > warn_report(), since IIUC, that'll get into HMP response correctly.
>
> Yes, that sounds better. I'll use that in v2, as well as tweaking the
> commit message.
>
> >
> >> +}
> >> endptr++;
> >> } else {
> >> mul = suffix_mul(default_suffix, unit);
> >
> > Regards,
> > Daniel
> >
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3226
> Virtualization: qemu.org | libvirt.org
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] docs: Add 'known_hosts_verify' parameter for libssh(2) connection uris
Signed-off-by: Jakob Meng Thanks for the review! Feel free to drop or change the URLs, e.g. to permalinks: https://gitlab.com/libvirt/libvirt/-/blob/f209d40a7e74e7e53a02c0c7ed20be218d390754/src/rpc/virnetsocket.c#L941 https://gitlab.com/libvirt/libvirt/-/blob/f209d40a7e74e7e53a02c0c7ed20be218d390754/src/rpc/virnetsocket.c#L1073 Jakob Am 05.02.21 um 12:52 schrieb Michal Privoznik: On 1/29/21 1:55 PM, Jakob Meng wrote: Parameter 'known_hosts_verify' is supported for some time now, but it is not yet documented. Ref.: https://gitlab.com/libvirt/libvirt/-/blob/master/src/rpc/virnetsocket.c#L941 https://gitlab.com/libvirt/libvirt/-/blob/master/src/rpc/virnetsocket.c#L1073 While these help with review I'd rather not put them into commit message because they are valid now, but as code shifts and move those lines might become stale. In fact, since the same code is in two places I think it should be de-duplicated. But that can be done in a follow up patch. However, you did not signed off your patch. We require patches to be signed off per https://libvirt.org/hacking.html#developer-certificate-of-origin No need to resend the patch, just reply to this e-mail with that line and I can amend it to the commit message and push. Michal OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 2/3] utils: Deprecate hex-with-suffix sizes
On 2/5/21 5:13 AM, Daniel P. Berrangé wrote:
> On Thu, Feb 04, 2021 at 01:07:07PM -0600, Eric Blake wrote:
>> Supporting '0x20M' looks odd, particularly since we have an 'E' suffix
>> that is ambiguous between a hex digit and the extremely large exibyte
>> suffix, as well as a 'B' suffix for bytes. In practice, people using
>> hex inputs are specifying values in bytes (and would have written
>> 0x200, or possibly relied on default_suffix in the case of
>> qemu_strtosz_MiB), and the use of scaling suffixes makes the most
>> sense for inputs in decimal (where the user would write 32M). But
>> rather than outright dropping support for hex-with-suffix, let's
>> follow our deprecation policy. Sadly, since qemu_strtosz() does not
>> have an Err** parameter, we pollute to stderr.
>
> Err** is only appropriate for errors, not warnings, so this statement
> can be removed.
The point of the comment was that we have no other mechanism for
reporting the deprecation other than polluting to stderr. And if we
ever remove the support, we'll either have to silently reject input that
we used to accept, or plumb through Err** handling to allow better
reporting of WHY we are rejecting input. But I didn't feel like adding
Err** support now.
>> @@ -309,6 +309,10 @@ static int do_strtosz(const char *nptr, const char
>> **end,
>> c = *endptr;
>> mul = suffix_mul(c, unit);
>> if (mul > 0) {
>> +if (hex) {
>> +fprintf(stderr, "Using a multiplier suffix on hex numbers "
>> +"is deprecated: %s\n", nptr);
>
> warn_report(), since IIUC, that'll get into HMP response correctly.
Yes, that sounds better. I'll use that in v2, as well as tweaking the
commit message.
>
>> +}
>> endptr++;
>> } else {
>> mul = suffix_mul(default_suffix, unit);
>
> Regards,
> Daniel
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH 2/3] utils: Deprecate hex-with-suffix sizes
On 2/5/21 4:25 AM, Vladimir Sementsov-Ogievskiy wrote:
> 04.02.2021 22:07, Eric Blake wrote:
>> Supporting '0x20M' looks odd, particularly since we have an 'E' suffix
>
> What about also deprecating 'E' suffix? (just my problem of reviewing
> previous patch)
No, we want to keep '1E' as a valid way to spell 1 exabyte. That has
uses in the wild (admittedly corner-case, as that is a LOT of storage).
It is only '0x1E' where the use of 'E' as a hex digit has priority over
'E' as a suffix meaning exabyte.
>
>> that is ambiguous between a hex digit and the extremely large exibyte
>> suffix, as well as a 'B' suffix for bytes. In practice, people using
>> hex inputs are specifying values in bytes (and would have written
>> 0x200, or possibly relied on default_suffix in the case of
>> qemu_strtosz_MiB), and the use of scaling suffixes makes the most
>> sense for inputs in decimal (where the user would write 32M). But
>> rather than outright dropping support for hex-with-suffix, let's
>> follow our deprecation policy. Sadly, since qemu_strtosz() does not
>> have an Err** parameter, we pollute to stderr.
>>
>> Signed-off-by: Eric Blake
>> ---
>> +++ b/util/cutils.c
>> @@ -264,7 +264,7 @@ static int do_strtosz(const char *nptr, const char
>> **end,
>> int retval;
>> const char *endptr;
>> unsigned char c;
>> - bool mul_required = false;
>> + bool mul_required = false, hex = false;
>> uint64_t val;
>> int64_t mul;
>> double fraction = 0.0;
>> @@ -309,6 +309,10 @@ static int do_strtosz(const char *nptr, const
>> char **end,
>
> you forget to set hex to true in corresponding if(){...}
>
>> c = *endptr;
>> mul = suffix_mul(c, unit);
>> if (mul > 0) {
>> + if (hex) {
>> + fprintf(stderr, "Using a multiplier suffix on hex numbers "
>> + "is deprecated: %s\n", nptr);
>> + }
D'oh. Now I get to rerun my tests to see when the warning triggers.
>> endptr++;
>> } else {
>> mul = suffix_mul(default_suffix, unit);
>
> should we also deprecate hex where default_suffix is not 'B' ?
That's exactly what this patch is (supposed to be) doing. If we parsed
a hex number, and there was an explicit suffix at all (which is
necessarily neither 'B' nor 'E', since those were already consumed while
parsing the hex number), issue a warning.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [libvirt PATCH v3 00/21] Add support for persistent mediated devices
On Tue, 2 Feb 2021 08:28:52 +0100 Erik Skultety wrote: > On Mon, Feb 01, 2021 at 04:38:56PM -0700, Alex Williamson wrote: > > On Mon, 1 Feb 2021 16:57:44 -0600 > > Jonathon Jongsma wrote: > > > > > On Mon, 1 Feb 2021 11:33:08 +0100 > > > Erik Skultety wrote: > > > > > > > On Mon, Feb 01, 2021 at 09:48:11AM +, Daniel P. Berrangé wrote: > > > > > On Mon, Feb 01, 2021 at 10:45:43AM +0100, Erik Skultety wrote: > > > > > > On Mon, Feb 01, 2021 at 09:42:32AM +, Daniel P. Berrangé > > > > > > wrote: > > > > > > > On Fri, Jan 29, 2021 at 05:34:36PM -0600, Jonathon Jongsma > > > > > > > wrote: > > > > > > > > On Thu, 7 Jan 2021 17:43:54 +0100 > > > > > > > > Erik Skultety wrote: > > > > > > > > > > > > > > > > > > Tested with v6.10.0-283-g1948d4e61e. > > > > > > > > > > > > > > > > > > > > 1.Can define/start/destroy mdev device successfully; > > > > > > > > > > > > > > > > > > > > 2.'virsh nodedev-list' has no '--active' option, which is > > > > > > > > > > inconsistent with the description in the patch： > > > > > > > > > > # virsh nodedev-list --active > > > > > > > > > > error: command 'nodedev-list' doesn't support option > > > > > > > > > > --active > > > > > > > > > > > > > > > > > > > > 3.virsh client hang when trying to destroy a mdev device > > > > > > > > > > which is using by a vm, and after that all 'virsh nodev*' > > > > > > > > > > cmds will hang. If restarting llibvirtd after that, > > > > > > > > > > libvirtd will hang. > > > > > > > > > > > > > > > > > > It hangs because underneath a write to the 'remove' sysfs > > > > > > > > > attribute is now blocking for some reason and since we're > > > > > > > > > relying on mdevctl to do it for us, hence "it hangs". I'm > > > > > > > > > not trying to make an excuse, it's plain wrong. I'd love to > > > > > > > > > rely on such a basic functionality, but it looks like we'll > > > > > > > > > have to go with a extremely ugly workaround and try to get > > > > > > > > > the list of active domains from the nodedev driver and see > > > > > > > > > whether any of them has the device assigned before we try > > > > > > > > > to destroy the mdev via the nodedev driver. > > > > > > > > > > > > > > > > So, I've been trying to figure out a way to do this, but as > > > > > > > > far as I know, there's no way to get a list of active domains > > > > > > > > from within the nodedev driver, and I can't think of any > > > > > > > > better ways to handle it. Any ideas? > > > > > > > > > > > > > > Correct, the nodedev driver isn't permitted to talk to any of > > > > > > > the virt drivers. > > > > > > > > > > > > Oh, not even via secondary connection? What makes nodedev so > > > > > > special, since we can open a secondary connection from e.g. the > > > > > > storage driver? > > > > > > > > > > It is technically possible, but it should never be done, because it > > > > > introduces a bi-directional dependancy between the daemons which > > > > > introduces the danger of deadlocking them. None of the secondary > > > > > drivers should connect to the hypervisor drivers. > > > > > > > > > > > > Is there anything in sysfs which reports whether the device is > > > > > > > in use ? > > > > > > > > > > > > Nothing that I know of, the way it used to work was that you > > > > > > tried to write to sysfs and kernel returned a write error with > > > > > > "device in use" or something like that, but that has changed > > > > > > since :(. > > > > > > > > Without having tried this and since mdevctl is just a Bash script, > > > > can we bypass mdevctl on destroys a little bit by constructing the > > > > path to the sysfs attribute ourselves and perform a non-blocking > > > > write of zero bytes to see if we get an error? If so, we'll skip > > > > mdevctl and report an error. If we don't, we'll invoke mdevctl to > > > > remove the device in order to remain consistent. Would that be an > > > > acceptable workaround (provided it would work)? > > > > > > As far as I can tell, this doesn't work. According to my > > > tests, attempting to write zero bytes to $(mdev_sysfs_path)/remove > > > doesn't result in an error if the mdev is in use by a vm. It just > > > "successfully" writes zero bytes. Adding Alex to cc in case he has an > > > idea for a workaround here. > > > > [Cc +Connie] > > > > I'm not really sure why mdevs are unique here. When we write to > > remove, the first step is to release the device from the driver, so > > it's really the same as an unbind for a vfio-pci device. PCI drivers > > cannot return an error, an unbind is handled not as a request, but a > > directive, so when the device is in use we block until the unbind can > > complete. With vfio-pci (and added upstream to the mdev core - > > depending on vendor support), the driver remove callback triggers a > > virtual interrupt to the user asking to cooperatively return the device > > (triggering a hot-unplug in
Re: [PATCH 00/10] Introduce virtio-mem model
On 2/5/21 9:34 AM, Jing Qi wrote: Thanks Michal. I checked the virtio_mem module in the host last time. I tried a new guest image with virtio_mem module and start the domain , then the value of actual can be set correctly. Yeah, it's the guest that needs the module. Glad to hear it's working. Michal
[libvirt PATCH] conf: allow virtio driver attributes for vhostuser disk
All of these options are actually supported by vhostuser disk so
we should allow them to be usable.
Signed-off-by: Pavel Hrdina
---
src/conf/domain_validate.c| 20 ---
.../disk-vhostuser.x86_64-latest.args | 4 ++--
tests/qemuxml2argvdata/disk-vhostuser.xml | 2 +-
.../disk-vhostuser.x86_64-latest.xml | 2 +-
4 files changed, 4 insertions(+), 24 deletions(-)
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 222a9386f6..6b3e892332 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -322,26 +322,6 @@ virDomainDiskVhostUserValidate(const virDomainDiskDef
*disk)
/* Unsupported driver elements */
-if (disk->virtio) {
-if (disk->virtio->iommu != VIR_TRISTATE_SWITCH_ABSENT) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("iommu is not supported with vhostuser disk"));
-return -1;
-}
-
-if (disk->virtio->ats != VIR_TRISTATE_SWITCH_ABSENT) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("ats is not supported with vhostuser disk"));
-return -1;
-}
-
-if (disk->virtio->packed != VIR_TRISTATE_SWITCH_ABSENT) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("packed is not supported with vhostuser disk"));
-return -1;
-}
-}
-
if (disk->src->metadataCacheMaxSize > 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("metadata_cache is not supported with vhostuser
disk"));
diff --git a/tests/qemuxml2argvdata/disk-vhostuser.x86_64-latest.args
b/tests/qemuxml2argvdata/disk-vhostuser.x86_64-latest.args
index b24b2c0b4f..30acd7c78b 100644
--- a/tests/qemuxml2argvdata/disk-vhostuser.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/disk-vhostuser.x86_64-latest.args
@@ -33,8 +33,8 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
-device vhost-user-blk-pci,bus=pci.0,addr=0x2,chardev=chr-vu-virtio-disk0,\
id=virtio-disk0,bootindex=1 \
-chardev socket,id=chr-vu-virtio-disk1,path=/tmp/vhost1.sock,reconnect=10 \
--device vhost-user-blk-pci,bus=pci.0,addr=0x3,chardev=chr-vu-virtio-disk1,\
-id=virtio-disk1 \
+-device vhost-user-blk-pci,iommu_platform=on,ats=on,packed=on,bus=pci.0,\
+addr=0x3,chardev=chr-vu-virtio-disk1,id=virtio-disk1 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 \
-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
resourcecontrol=deny \
diff --git a/tests/qemuxml2argvdata/disk-vhostuser.xml
b/tests/qemuxml2argvdata/disk-vhostuser.xml
index c96ef9119c..cbe2804e39 100644
--- a/tests/qemuxml2argvdata/disk-vhostuser.xml
+++ b/tests/qemuxml2argvdata/disk-vhostuser.xml
@@ -20,7 +20,7 @@
-
+
diff --git a/tests/qemuxml2xmloutdata/disk-vhostuser.x86_64-latest.xml
b/tests/qemuxml2xmloutdata/disk-vhostuser.x86_64-latest.xml
index 9712dc0b12..87f5ec46ac 100644
--- a/tests/qemuxml2xmloutdata/disk-vhostuser.x86_64-latest.xml
+++ b/tests/qemuxml2xmloutdata/disk-vhostuser.x86_64-latest.xml
@@ -28,7 +28,7 @@
-
+
--
2.29.2
Re: [PATCH] docs: Add 'known_hosts_verify' parameter for libssh(2) connection uris
On 1/29/21 1:55 PM, Jakob Meng wrote: Parameter 'known_hosts_verify' is supported for some time now, but it is not yet documented. Ref.: https://gitlab.com/libvirt/libvirt/-/blob/master/src/rpc/virnetsocket.c#L941 https://gitlab.com/libvirt/libvirt/-/blob/master/src/rpc/virnetsocket.c#L1073 While these help with review I'd rather not put them into commit message because they are valid now, but as code shifts and move those lines might become stale. In fact, since the same code is in two places I think it should be de-duplicated. But that can be done in a follow up patch. However, you did not signed off your patch. We require patches to be signed off per https://libvirt.org/hacking.html#developer-certificate-of-origin No need to resend the patch, just reply to this e-mail with that line and I can amend it to the commit message and push. Michal
Re: [PATCH] qemuDomainAttachRedirdevDevice: Remove need_release variable
On 2/3/21 7:18 AM, Yi Li wrote: Get rid of the 'need_release' variable. Signed-off-by: Yi Li --- src/qemu/qemu_hotplug.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) Reviewed-by: Michal Privoznik and pushed. Michal
Re: [PATCH 2/3] utils: Deprecate hex-with-suffix sizes
On Thu, Feb 04, 2021 at 01:07:07PM -0600, Eric Blake wrote:
> Supporting '0x20M' looks odd, particularly since we have an 'E' suffix
> that is ambiguous between a hex digit and the extremely large exibyte
> suffix, as well as a 'B' suffix for bytes. In practice, people using
> hex inputs are specifying values in bytes (and would have written
> 0x200, or possibly relied on default_suffix in the case of
> qemu_strtosz_MiB), and the use of scaling suffixes makes the most
> sense for inputs in decimal (where the user would write 32M). But
> rather than outright dropping support for hex-with-suffix, let's
> follow our deprecation policy. Sadly, since qemu_strtosz() does not
> have an Err** parameter, we pollute to stderr.
Err** is only appropriate for errors, not warnings, so this statement
can be removed.
>
> Signed-off-by: Eric Blake
> ---
> docs/system/deprecated.rst | 8
> util/cutils.c | 6 +-
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index 6ac757ed9fa7..c404c3926e6f 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -146,6 +146,14 @@ library enabled as a cryptography provider.
> Neither the ``nettle`` library, or the built-in cryptography provider are
> supported on FIPS enabled hosts.
>
> +hexadecimal sizes with scaling multipliers (since 6.0)
> +''
> +
> +Input parameters that take a size value should only use a size suffix
> +(such as 'k' or 'M') when the base is written in decimal, and not when
> +the value is hexadecimal. That is, '0x20M' is deprecated, and should
> +be written either as '32M' or as '0x200'.
> +
> QEMU Machine Protocol (QMP) commands
>
>
> diff --git a/util/cutils.c b/util/cutils.c
> index 0234763bd70b..75190565cbb5 100644
> --- a/util/cutils.c
> +++ b/util/cutils.c
> @@ -264,7 +264,7 @@ static int do_strtosz(const char *nptr, const char **end,
> int retval;
> const char *endptr;
> unsigned char c;
> -bool mul_required = false;
> +bool mul_required = false, hex = false;
> uint64_t val;
> int64_t mul;
> double fraction = 0.0;
> @@ -309,6 +309,10 @@ static int do_strtosz(const char *nptr, const char **end,
> c = *endptr;
> mul = suffix_mul(c, unit);
> if (mul > 0) {
> +if (hex) {
> +fprintf(stderr, "Using a multiplier suffix on hex numbers "
> +"is deprecated: %s\n", nptr);
warn_report(), since IIUC, that'll get into HMP response correctly.
> +}
> endptr++;
> } else {
> mul = suffix_mul(default_suffix, unit);
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] Remove the deprecated pc-1.x machine types and related stuff
On Wed, Feb 03, 2021 at 06:18:28PM +0100, Thomas Huth wrote: > The pc-1.x machine types have been deprecated since QEMU v5.0 already, and > nobody complained, so they can now be removed. While we're at it, also > remove some compatibility switches and related code that are now not > necessary anymore after these machine types have been removed. > (We could maybe even remove more stuff like the various host_features > switches in the virtio devices, but they still might be useful in certain > cases, so I decided not to remove them yet) I queued patches 1 and 3. Thanks! > Thomas Huth (4): > hw/i386: Remove the deprecated pc-1.x machine types > hw/block/fdc: Remove the check_media_rate property > hw/virtio/virtio-balloon: Remove the "class" property > hw/usb/bus: Remove the "full-path" property > > docs/system/deprecated.rst | 6 -- > docs/system/removed-features.rst | 6 ++ > hw/block/fdc.c | 17 +- > hw/i386/pc_piix.c| 94 > hw/usb/bus.c | 7 +-- > hw/virtio/virtio-balloon-pci.c | 11 +--- > include/hw/usb.h | 2 +- > tests/qemu-iotests/172.out | 35 > 8 files changed, 11 insertions(+), 167 deletions(-) > > -- > 2.27.0
Re: [PATCH 2/3] utils: Deprecate hex-with-suffix sizes
On Fri, Feb 05, 2021 at 01:25:18PM +0300, Vladimir Sementsov-Ogievskiy wrote: > 04.02.2021 22:07, Eric Blake wrote: > >Supporting '0x20M' looks odd, particularly since we have an 'E' suffix > > What about also deprecating 'E' suffix? (just my problem of reviewing > previous patch) Ha! What if people want to specify exabytes?! That actually works at the moment: $ nbdkit memory 7E --run 'qemu-io -f raw -c "r -v 1E 512" "$uri"' Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
[libvirt PATCH] tests: Only mock $INODE64 symbols on x86_64 macOS
The version of macOS running on Apple Silicon doesn't need to
concern itself with backwards compatibility with 32-bit
applications, and so it could jettison all the symbol aliasing
shenanigans involved.
https://gitlab.com/libvirt/libvirt/-/issues/121
Signed-off-by: Andrea Bolognani
---
tests/virfilewrapper.c | 2 +-
tests/virmockstathelpers.c | 4 ++--
tests/virpcimock.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/virfilewrapper.c b/tests/virfilewrapper.c
index ca2356b5c9..1369cfb766 100644
--- a/tests/virfilewrapper.c
+++ b/tests/virfilewrapper.c
@@ -56,7 +56,7 @@ static void init_syms(void)
VIR_MOCK_REAL_INIT(access);
VIR_MOCK_REAL_INIT(mkdir);
VIR_MOCK_REAL_INIT(open);
-# ifdef __APPLE__
+# if defined(__APPLE__) && defined(__x86_64__)
VIR_MOCK_REAL_INIT_ALIASED(opendir, "opendir$INODE64");
# else
VIR_MOCK_REAL_INIT(opendir);
diff --git a/tests/virmockstathelpers.c b/tests/virmockstathelpers.c
index 830dfe1085..9344345baa 100644
--- a/tests/virmockstathelpers.c
+++ b/tests/virmockstathelpers.c
@@ -161,7 +161,7 @@ static void virMockStatInit(void)
debug = getenv("VIR_MOCK_STAT_DEBUG");
#ifdef MOCK_STAT
-# ifdef __APPLE__
+# if defined(__APPLE__) && defined(__x86_64__)
VIR_MOCK_REAL_INIT_ALIASED(stat, "stat$INODE64");
# else
VIR_MOCK_REAL_INIT(stat);
@@ -181,7 +181,7 @@ static void virMockStatInit(void)
fdebug("real __xstat64 %p\n", real___xstat64);
#endif
#ifdef MOCK_LSTAT
-# ifdef __APPLE__
+# if defined(__APPLE__) && defined(__x86_64__)
VIR_MOCK_REAL_INIT_ALIASED(lstat, "lstat$INODE64");
# else
VIR_MOCK_REAL_INIT(lstat);
diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index f6280fc8b5..d1c6220c57 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -936,7 +936,7 @@ init_syms(void)
VIR_MOCK_REAL_INIT(__open_2);
# endif /* ! __GLIBC__ */
VIR_MOCK_REAL_INIT(close);
-# ifdef __APPLE__
+# if defined(__APPLE__) && defined(__x86_64__)
VIR_MOCK_REAL_INIT_ALIASED(opendir, "opendir$INODE64");
# else
VIR_MOCK_REAL_INIT(opendir);
--
2.26.2
Re: [libvirt PATCH] Revert "tests: Avoid gnulib replacements in mocks"
On Thu, Feb 04, 2021 at 11:48:26AM +0100, Andrea Bolognani wrote:
> Now that we're no longer using gnulib, we can treat macOS the
> same as all other targets.
>
> This reverts commit 0ae6f5cea54d95c0d1dedf04a0a2accfe2529fb2
>
> Signed-off-by: Andrea Bolognani
> ---
> Roman,
>
> this seems to work fine both locally and on Cirrus CI[1], but
> I'd like to have your thumbs up before pushing.
>
Hi,
I thought about that too back in Oct/Nov. Thanks for doing the change!
Reviewed-by: Roman Bolshakov
Tested-by: Roman Bolshakov
Regards,
Roman
> Thanks!
>
> [1] https://gitlab.com/abologna/libvirt/-/jobs/1007362310
>
> tests/virfilewrapper.c | 5 -
> tests/virmockstathelpers.c | 10 --
> 2 files changed, 15 deletions(-)
>
> diff --git a/tests/virfilewrapper.c b/tests/virfilewrapper.c
> index ca2356b5c9..7034e22420 100644
> --- a/tests/virfilewrapper.c
> +++ b/tests/virfilewrapper.c
> @@ -163,12 +163,7 @@ int access(const char *path, int mode)
> return real_access(newpath ? newpath : path, mode);
> }
>
> -# ifdef __APPLE__
> -int _open(const char *path, int flags, ...) __asm("_open");
> -int _open(const char *path, int flags, ...)
> -# else
> int open(const char *path, int flags, ...)
> -# endif
> {
> g_autofree char *newpath = NULL;
> va_list ap;
> diff --git a/tests/virmockstathelpers.c b/tests/virmockstathelpers.c
> index 3bd2437ffe..49485cd360 100644
> --- a/tests/virmockstathelpers.c
> +++ b/tests/virmockstathelpers.c
> @@ -204,12 +204,7 @@ static int virMockStatRedirect(const char *path, char
> **newpath);
> #endif
>
> #ifdef MOCK_STAT
> -# ifdef __APPLE__
> -int _stat(const char *path, struct stat *sb) __asm("_stat$INODE64");
> -int _stat(const char *path, struct stat *sb)
> -# else
> int stat(const char *path, struct stat *sb)
> -# endif
> {
> g_autofree char *newpath = NULL;
>
> @@ -279,13 +274,8 @@ __xstat64(int ver, const char *path, struct stat64 *sb)
> #endif
>
> #ifdef MOCK_LSTAT
> -# ifdef __APPLE__
> -int _lstat(const char *path, struct stat *sb) __asm("_lstat$INODE64");
> -int _lstat(const char *path, struct stat *sb)
> -# else
> int
> lstat(const char *path, struct stat *sb)
> -# endif
> {
> g_autofree char *newpath = NULL;
>
> --
> 2.26.2
>
Re: [PATCH 4/4] hw/usb/bus: Remove the "full-path" property
On Thu, Feb 04, 2021 at 04:51:39PM +0100, Thomas Huth wrote:
> On 04/02/2021 09.36, Gerd Hoffmann wrote:
> >Hi,
> >
> > > enum USBDeviceFlags {
> > > -USB_DEV_FLAG_FULL_PATH,
> > > +USB_DEV_FLAG_FULL_PATH, /* unused since QEMU v6.0 */
> >
> > Why not just drop it? Any remaining users?
>
> I didn't want to change the values of the other members of the enum ...
This should be purely internal to qemu hw/usb and not some kind of abi,
so changing the values shouldn't break anything ...
take care,
Gerd
Re: [PATCH] util: Remove '\n' from vhostuser ifname
On 2/5/21 4:10 AM, Yalei Li wrote: When deleting the vhostuserclient interface, OVS prompts that the interface does not exist, Through the XML file, I found that the "target dev" has a '\n', results in an XML parsing error. XML file: That is because 'ovs-vsctl' returns a newline result, always come with a '\n', and the vircommandrun function puts it in ifname. So virNetDevOpenvswitchGetVhostuserIfname should remove '\n' from ifname. Signed-off-by: Yalei Li --- src/util/virnetdevopenvswitch.c | 1 + 1 file changed, 1 insertion(+) Oh sorry, I saw you posted this two weeks ago and had it marked for review but then got distracted. I've added Daniel's reviewed by tag, added mine and pushed. Reviewed-by: Michal Privoznik Congratulations on your first libvirt contribution! Michal
Re: [PATCH] docs: Remove broken link to Xen channel doc
On 2/5/21 12:45 AM, Jim Fehlig wrote: Many of Xen's text documents have been converted to man pages over the years, the channel doc being one of them. Replace the broken channel.txt link with the name of the man page providing the same information. Signed-off-by: Jim Fehlig --- docs/formatdomain.rst | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) Reviewed-by: Michal Privoznik Michal
Re: [PATCH] bhyve: relax emulator binary value check
On 2/4/21 4:47 PM, Roman Bogorodskiy wrote: Currently, requesting domain capabilities fails when the specified emulator binary does not equal to "/usr/sbin/bhyve". Relax this check to allow any path with basename "bhyve", as it's a common case when binary is specified without an absolute path. Signed-off-by: Roman Bogorodskiy --- I'm not sure how useful is this check in general: at this point we don't use the user specified emulator value anyway, just use BHYVE constant. Probably it would be better to just drop the entire "else" block? Yes, I was just about to suggest that. We don't do any check like this for QEMU driver. Just execute user provided binary (with sume arguments appended to get QMP monitor) and query caps. Looking into bhyve driver code though, it doesn't use from domain XML, does it? What I'm getting at is that there is no way for a user to specify different binary to run than /usr/sbin/bhyve. So it doesn't really make sense to provide emulatorbin at all. Since I can argue both ways, merge this patch or just remove the block completely. Reviewed-by: Michal Privoznik Michal
Re: [PATCH 00/10] Introduce virtio-mem model
Thanks Michal. I checked the virtio_mem module in the host last time. I tried a new guest image with virtio_mem module and start the domain , then the value of actual can be set correctly. Jing Qi On Thu, Feb 4, 2021 at 4:52 PM Michal Privoznik wrote: > On 2/4/21 3:33 AM, Jing Qi wrote: > > Michal, > > I checked the virtio_mem module and it's loaded - > > > > lsmod |grep virtio_mem > > virtio_mem 32768 0 > > > > And I can't make the actual value change to non-zerio. > > -> virsh update-memory pc --requested-size 256M > > or > > ->virsh setmem pc 1000M > > This is unrelated. 'setmem' modifies balloon not virtio-mem-pci device. > > > > > > > > > 524288 > > 0 > > 2048 > > 262144 > > 0 > > > > > > > function='0x0'/> > > > > Any other suggestions ? > > > Is there something in the guest dmesg? This is what I get: > > virsh update-memory-device gentoo --alias ua-virtiomem 256M > > [ 17.619060] virtio_mem virtio3: plugged size: 0x0 > [ 17.619062] virtio_mem virtio3: requested size: 0x1000 > [ 17.653072] Built 4 zonelists, mobility grouping on. Total pages: > 2065850 > [ 17.653074] Policy zone: Normal > > > And I can see actual size updated: > > > > 2048 > > > 4194304 > 0 > 2048 > 262144 > 262144 > > > function='0x0'/> > > > Maybe David knows the answer. > > Michal > > -- Thanks & Regards, Jing,Qi
