Re: Questions about how block devices use snapshots
Hi Kevin, Thank you for your reply and detailed answers. In my scenario is the iSCSI SAN environment. The network storage device is connected to the physical machine via iSCSI, and LVM is used as the middle layer between the storage device and the VM for storage management and metadata synchronization. Every VM uses both raw and qcow2 formats, with the system disk being qcow2 and the data disk being raw. Therefore block devices need to support snapshot capability in both raw and qcow2 store methods. In addition, snapshot images should also be stored in iSCSI storage, which is a block device. Both internal and external snapshots can implement snapshots of block devices, but they both have their drawbacks when multiple snapshots are required. Internal snapshots can only be used in qcow2 format and do not require additional creation of new block devices. As you said, the block device has much more space than the virtual disk. There is no telling when disk space will be full when creating multiple snapshots. External snapshots require the creation of additional block devices to store the overlay images, but it is not clear how much space needs to be created. If the space is the same as the virtual disk, when there are multiple snapshots it will be a serious waste of disk space, because each time a new snapshot is created the previous one will become read-only. However, if the disk space created is too small, the snapshot data may not be stored when the disk space is full. The problem with both is the uncertainty of the space size of the block device at the time of creation. Of course, we can rely on lvm's resize function to dynamically grow the space of the block device. But I think this is more of a workaround. It is mentioned in the Qemu docs page under "QEMU disk image utility" that the qemu-img rebase can be used to perform a “diff” operation on two disk images. Say that base.img has been cloned as modified.img by copying it, and that the modified.img guest has run so there are now some changes compared to base.img. To construct a thin image called diff.qcow2 that contains just the differences, do: qemu-img create -f qcow2 -b modified.img diff.qcow2 qemu-img rebase -b base.img diff.qcow2 At this point, modified.img can be discarded, since base.img + diff.qcow2 contains the same information. Can this “diff” operation be used on snapshots of block devices? The first snapshot is a copy of the original disk (to save space we can copy only the data that has already been used), while the subsequent snapshots are based on the diff of the previous snapshot, so that the space required for the created block device is known at the time of the snapshot. Regards Zhiyong On 1/9/23 9:57 PM, Kevin Wolf wrote: Am 09.01.2023 um 13:45 hat Zhiyong Ye geschrieben: Qemu provides powerful snapshot capabilities for different file formats. But this is limited to the block backend being a file, and support is not good enough when it is a block device. When creating snapshots based on files, there is no need to specify the size of the snapshot image, which can grow dynamically as the virtual machine is used. But block devices are fixed in size at creation and cannot be dynamically grown at a later time. So is there any way to support snapshots when the block backend is a block device? In order to have snapshots, you need to have an image format like qcow2. A qcow2 file can have a raw block device as its backing file, so even if you store the overlay image on a filesystem, you have technically snapshotted a block device. This may or may not be enough for your use case. It is also possible to store qcow2 files on block devices, though depending on your requirements, it can get very tricky because then you're responsible for making sure that there is always enough free space on the block device. So a second, still very simple, approach could be taking a second block device that is a little bit larger than the virtual disk (for the qcow2 metadata) and use that as the external snapshot. Obviously, you require a lot of disk space this way, because each snapshots needs to be able to store the full image. You could also use internal snapshots. In this case, you just need to make sure that the block device is a lot larger than the virtual disk, so that there is enough space left for storing the snapshots. At some point it will be full. And finally, for example if your block devices are actually LVs, you could start resizing the block device dynmically as needed. This becomes very complex quickly and you're on your own, but it is possible and has been done by oVirt. Kevin
[PULL 5/6] hw/nvme: clean up confusing use of errp/local_err
From: Klaus Jensen
Remove an unnecessary local Error value in nvme_realize(). In the
process, change nvme_check_constraints() to return a bool.
Reviewed-by: Markus Armbruster
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Klaus Jensen
---
hw/nvme/ctrl.c | 48 +++-
1 file changed, 23 insertions(+), 25 deletions(-)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 226480033771..b21455ada660 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -6981,7 +6981,7 @@ static const MemoryRegionOps nvme_cmb_ops = {
},
};
-static void nvme_check_constraints(NvmeCtrl *n, Error **errp)
+static bool nvme_check_params(NvmeCtrl *n, Error **errp)
{
NvmeParams *params = >params;
@@ -6995,38 +6995,38 @@ static void nvme_check_constraints(NvmeCtrl *n, Error
**errp)
if (n->namespace.blkconf.blk && n->subsys) {
error_setg(errp, "subsystem support is unavailable with legacy "
"namespace ('drive' property)");
-return;
+return false;
}
if (params->max_ioqpairs < 1 ||
params->max_ioqpairs > NVME_MAX_IOQPAIRS) {
error_setg(errp, "max_ioqpairs must be between 1 and %d",
NVME_MAX_IOQPAIRS);
-return;
+return false;
}
if (params->msix_qsize < 1 ||
params->msix_qsize > PCI_MSIX_FLAGS_QSIZE + 1) {
error_setg(errp, "msix_qsize must be between 1 and %d",
PCI_MSIX_FLAGS_QSIZE + 1);
-return;
+return false;
}
if (!params->serial) {
error_setg(errp, "serial property not set");
-return;
+return false;
}
if (n->pmr.dev) {
if (host_memory_backend_is_mapped(n->pmr.dev)) {
error_setg(errp, "can't use already busy memdev: %s",
object_get_canonical_path_component(OBJECT(n->pmr.dev)));
-return;
+return false;
}
if (!is_power_of_2(n->pmr.dev->size)) {
error_setg(errp, "pmr backend size needs to be power of 2 in
size");
-return;
+return false;
}
host_memory_backend_set_mapped(n->pmr.dev, true);
@@ -7035,64 +7035,64 @@ static void nvme_check_constraints(NvmeCtrl *n, Error
**errp)
if (n->params.zasl > n->params.mdts) {
error_setg(errp, "zoned.zasl (Zone Append Size Limit) must be less "
"than or equal to mdts (Maximum Data Transfer Size)");
-return;
+return false;
}
if (!n->params.vsl) {
error_setg(errp, "vsl must be non-zero");
-return;
+return false;
}
if (params->sriov_max_vfs) {
if (!n->subsys) {
error_setg(errp, "subsystem is required for the use of SR-IOV");
-return;
+return false;
}
if (params->sriov_max_vfs > NVME_MAX_VFS) {
error_setg(errp, "sriov_max_vfs must be between 0 and %d",
NVME_MAX_VFS);
-return;
+return false;
}
if (params->cmb_size_mb) {
error_setg(errp, "CMB is not supported with SR-IOV");
-return;
+return false;
}
if (n->pmr.dev) {
error_setg(errp, "PMR is not supported with SR-IOV");
-return;
+return false;
}
if (!params->sriov_vq_flexible || !params->sriov_vi_flexible) {
error_setg(errp, "both sriov_vq_flexible and sriov_vi_flexible"
" must be set for the use of SR-IOV");
-return;
+return false;
}
if (params->sriov_vq_flexible < params->sriov_max_vfs * 2) {
error_setg(errp, "sriov_vq_flexible must be greater than or equal"
" to %d (sriov_max_vfs * 2)", params->sriov_max_vfs *
2);
-return;
+return false;
}
if (params->max_ioqpairs < params->sriov_vq_flexible + 2) {
error_setg(errp, "(max_ioqpairs - sriov_vq_flexible) must be"
" greater than or equal to 2");
-return;
+return false;
}
if (params->sriov_vi_flexible < params->sriov_max_vfs) {
error_setg(errp, "sriov_vi_flexible must be greater than or equal"
" to %d (sriov_max_vfs)", params->sriov_max_vfs);
-return;
+return false;
}
if (params->msix_qsize < params->sriov_vi_flexible + 1) {
error_setg(errp, "(msix_qsize - sriov_vi_flexible) must be"
" greater than or equal to 1");
-return;
+return false;
}
if (params->sriov_max_vi_per_vf &&
@@ -7100,7 +7100,7 @@ static void nvme_check_constraints(NvmeCtrl *n, Error
**errp)
error_setg(errp, "sriov_max_vi_per_vf must meet:"
[PULL 6/6] hw/nvme: cleanup error reporting in nvme_init_pci()
From: Klaus Jensen
Replace the local Error variable with errp and ERRP_GUARD() and change
the return value to bool.
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Klaus Jensen
---
hw/nvme/ctrl.c | 25 -
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index b21455ada660..f25cc2c235e9 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -7290,15 +7290,14 @@ static int nvme_add_pm_capability(PCIDevice *pci_dev,
uint8_t offset)
return 0;
}
-static int nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev, Error **errp)
+static bool nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev, Error **errp)
{
+ERRP_GUARD();
uint8_t *pci_conf = pci_dev->config;
uint64_t bar_size;
unsigned msix_table_offset, msix_pba_offset;
int ret;
-Error *err = NULL;
-
pci_conf[PCI_INTERRUPT_PIN] = 1;
pci_config_set_prog_interface(pci_conf, 0x2);
@@ -7335,14 +7334,14 @@ static int nvme_init_pci(NvmeCtrl *n, PCIDevice
*pci_dev, Error **errp)
}
ret = msix_init(pci_dev, n->params.msix_qsize,
>bar0, 0, msix_table_offset,
->bar0, 0, msix_pba_offset, 0, );
-if (ret < 0) {
-if (ret == -ENOTSUP) {
-warn_report_err(err);
-} else {
-error_propagate(errp, err);
-return ret;
-}
+>bar0, 0, msix_pba_offset, 0, errp);
+if (ret == -ENOTSUP) {
+/* report that msix is not supported, but do not error out */
+warn_report_err(*errp);
+*errp = NULL;
+} else if (ret < 0) {
+/* propagate error to caller */
+return false;
}
nvme_update_msixcap_ts(pci_dev, n->conf_msix_qsize);
@@ -7359,7 +7358,7 @@ static int nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev,
Error **errp)
nvme_init_sriov(n, pci_dev, 0x120);
}
-return 0;
+return true;
}
static void nvme_init_subnqn(NvmeCtrl *n)
@@ -7535,7 +7534,7 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
return;
}
nvme_init_state(n);
-if (nvme_init_pci(n, pci_dev, errp)) {
+if (!nvme_init_pci(n, pci_dev, errp)) {
return;
}
nvme_init_ctrl(n, pci_dev);
--
2.39.0
[PULL 4/6] hw/nvme: fix missing cq eventidx update
From: Klaus Jensen
Prior to reading the shadow doorbell cq head, we have to update the
eventidx. Otherwise, we risk that the driver will skip an mmio doorbell
write. This happens on riscv64, as reported by Guenter.
Adding the missing update to the cq eventidx fixes the issue.
Fixes: 3f7fe8de3d49 ("hw/nvme: Implement shadow doorbell buffer support")
Cc: qemu-sta...@nongnu.org
Cc: qemu-ri...@nongnu.org
Reported-by: Guenter Roeck
Reviewed-by: Keith Busch
Signed-off-by: Klaus Jensen
---
hw/nvme/ctrl.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 28e02ec7baa6..226480033771 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -1334,6 +1334,15 @@ static inline void nvme_blk_write(BlockBackend *blk,
int64_t offset,
}
}
+static void nvme_update_cq_eventidx(const NvmeCQueue *cq)
+{
+uint32_t v = cpu_to_le32(cq->head);
+
+trace_pci_nvme_update_cq_eventidx(cq->cqid, cq->head);
+
+pci_dma_write(PCI_DEVICE(cq->ctrl), cq->ei_addr, , sizeof(v));
+}
+
static void nvme_update_cq_head(NvmeCQueue *cq)
{
uint32_t v;
@@ -1358,6 +1367,7 @@ static void nvme_post_cqes(void *opaque)
hwaddr addr;
if (n->dbbuf_enabled) {
+nvme_update_cq_eventidx(cq);
nvme_update_cq_head(cq);
}
--
2.39.0
[PULL 3/6] hw/nvme: fix missing endian conversions for doorbell buffers
From: Klaus Jensen
The eventidx and doorbell value are not handling endianness correctly.
Fix this.
Fixes: 3f7fe8de3d49 ("hw/nvme: Implement shadow doorbell buffer support")
Cc: qemu-sta...@nongnu.org
Reported-by: Guenter Roeck
Reviewed-by: Keith Busch
Signed-off-by: Klaus Jensen
---
hw/nvme/ctrl.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index cfe16476f0a4..28e02ec7baa6 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -1336,8 +1336,11 @@ static inline void nvme_blk_write(BlockBackend *blk,
int64_t offset,
static void nvme_update_cq_head(NvmeCQueue *cq)
{
-pci_dma_read(PCI_DEVICE(cq->ctrl), cq->db_addr, >head,
- sizeof(cq->head));
+uint32_t v;
+
+pci_dma_read(PCI_DEVICE(cq->ctrl), cq->db_addr, , sizeof(v));
+
+cq->head = le32_to_cpu(v);
trace_pci_nvme_update_cq_head(cq->cqid, cq->head);
}
@@ -6148,16 +6151,20 @@ static uint16_t nvme_admin_cmd(NvmeCtrl *n, NvmeRequest
*req)
static void nvme_update_sq_eventidx(const NvmeSQueue *sq)
{
+uint32_t v = cpu_to_le32(sq->tail);
+
trace_pci_nvme_update_sq_eventidx(sq->sqid, sq->tail);
-pci_dma_write(PCI_DEVICE(sq->ctrl), sq->ei_addr, >tail,
- sizeof(sq->tail));
+pci_dma_write(PCI_DEVICE(sq->ctrl), sq->ei_addr, , sizeof(v));
}
static void nvme_update_sq_tail(NvmeSQueue *sq)
{
-pci_dma_read(PCI_DEVICE(sq->ctrl), sq->db_addr, >tail,
- sizeof(sq->tail));
+uint32_t v;
+
+pci_dma_read(PCI_DEVICE(sq->ctrl), sq->db_addr, , sizeof(v));
+
+sq->tail = le32_to_cpu(v);
trace_pci_nvme_update_sq_tail(sq->sqid, sq->tail);
}
--
2.39.0
[PULL 1/6] hw/nvme: use QOM accessors
From: Klaus Jensen
Replace various ->parent_obj use with the equivalent QOM accessors.
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Klaus Jensen
---
hw/nvme/ctrl.c | 89 +++---
1 file changed, 48 insertions(+), 41 deletions(-)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 4a0c51a9477e..78c2f4e39d0a 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -449,7 +449,7 @@ static int nvme_addr_read(NvmeCtrl *n, hwaddr addr, void
*buf, int size)
return 0;
}
-return pci_dma_read(>parent_obj, addr, buf, size);
+return pci_dma_read(PCI_DEVICE(n), addr, buf, size);
}
static int nvme_addr_write(NvmeCtrl *n, hwaddr addr, const void *buf, int size)
@@ -469,7 +469,7 @@ static int nvme_addr_write(NvmeCtrl *n, hwaddr addr, const
void *buf, int size)
return 0;
}
-return pci_dma_write(>parent_obj, addr, buf, size);
+return pci_dma_write(PCI_DEVICE(n), addr, buf, size);
}
static bool nvme_nsid_valid(NvmeCtrl *n, uint32_t nsid)
@@ -514,24 +514,27 @@ static uint8_t nvme_sq_empty(NvmeSQueue *sq)
static void nvme_irq_check(NvmeCtrl *n)
{
+PCIDevice *pci = PCI_DEVICE(n);
uint32_t intms = ldl_le_p(>bar.intms);
-if (msix_enabled(&(n->parent_obj))) {
+if (msix_enabled(pci)) {
return;
}
if (~intms & n->irq_status) {
-pci_irq_assert(>parent_obj);
+pci_irq_assert(pci);
} else {
-pci_irq_deassert(>parent_obj);
+pci_irq_deassert(pci);
}
}
static void nvme_irq_assert(NvmeCtrl *n, NvmeCQueue *cq)
{
+PCIDevice *pci = PCI_DEVICE(n);
+
if (cq->irq_enabled) {
-if (msix_enabled(&(n->parent_obj))) {
+if (msix_enabled(pci)) {
trace_pci_nvme_irq_msix(cq->vector);
-msix_notify(&(n->parent_obj), cq->vector);
+msix_notify(pci, cq->vector);
} else {
trace_pci_nvme_irq_pin();
assert(cq->vector < 32);
@@ -546,7 +549,7 @@ static void nvme_irq_assert(NvmeCtrl *n, NvmeCQueue *cq)
static void nvme_irq_deassert(NvmeCtrl *n, NvmeCQueue *cq)
{
if (cq->irq_enabled) {
-if (msix_enabled(&(n->parent_obj))) {
+if (msix_enabled(PCI_DEVICE(n))) {
return;
} else {
assert(cq->vector < 32);
@@ -570,7 +573,7 @@ static void nvme_req_clear(NvmeRequest *req)
static inline void nvme_sg_init(NvmeCtrl *n, NvmeSg *sg, bool dma)
{
if (dma) {
-pci_dma_sglist_init(>qsg, >parent_obj, 0);
+pci_dma_sglist_init(>qsg, PCI_DEVICE(n), 0);
sg->flags = NVME_SG_DMA;
} else {
qemu_iovec_init(>iov, 0);
@@ -1333,7 +1336,7 @@ static inline void nvme_blk_write(BlockBackend *blk,
int64_t offset,
static void nvme_update_cq_head(NvmeCQueue *cq)
{
-pci_dma_read(>ctrl->parent_obj, cq->db_addr, >head,
+pci_dma_read(PCI_DEVICE(cq->ctrl), cq->db_addr, >head,
sizeof(cq->head));
trace_pci_nvme_shadow_doorbell_cq(cq->cqid, cq->head);
}
@@ -1363,7 +1366,7 @@ static void nvme_post_cqes(void *opaque)
req->cqe.sq_id = cpu_to_le16(sq->sqid);
req->cqe.sq_head = cpu_to_le16(sq->head);
addr = cq->dma_addr + cq->tail * n->cqe_size;
-ret = pci_dma_write(>parent_obj, addr, (void *)>cqe,
+ret = pci_dma_write(PCI_DEVICE(n), addr, (void *)>cqe,
sizeof(req->cqe));
if (ret) {
trace_pci_nvme_err_addr_write(addr);
@@ -4615,6 +4618,7 @@ static uint16_t nvme_get_log(NvmeCtrl *n, NvmeRequest
*req)
static void nvme_free_cq(NvmeCQueue *cq, NvmeCtrl *n)
{
+PCIDevice *pci = PCI_DEVICE(n);
uint16_t offset = (cq->cqid << 3) + (1 << 2);
n->cq[cq->cqid] = NULL;
@@ -4625,8 +4629,8 @@ static void nvme_free_cq(NvmeCQueue *cq, NvmeCtrl *n)
event_notifier_set_handler(>notifier, NULL);
event_notifier_cleanup(>notifier);
}
-if (msix_enabled(>parent_obj)) {
-msix_vector_unuse(>parent_obj, cq->vector);
+if (msix_enabled(pci)) {
+msix_vector_unuse(pci, cq->vector);
}
if (cq->cqid) {
g_free(cq);
@@ -4664,8 +4668,10 @@ static void nvme_init_cq(NvmeCQueue *cq, NvmeCtrl *n,
uint64_t dma_addr,
uint16_t cqid, uint16_t vector, uint16_t size,
uint16_t irq_enabled)
{
-if (msix_enabled(>parent_obj)) {
-msix_vector_use(>parent_obj, vector);
+PCIDevice *pci = PCI_DEVICE(n);
+
+if (msix_enabled(pci)) {
+msix_vector_use(pci, vector);
}
cq->ctrl = n;
cq->cqid = cqid;
@@ -4716,7 +4722,7 @@ static uint16_t nvme_create_cq(NvmeCtrl *n, NvmeRequest
*req)
trace_pci_nvme_err_invalid_create_cq_addr(prp1);
return NVME_INVALID_PRP_OFFSET | NVME_DNR;
}
-if (unlikely(!msix_enabled(>parent_obj) && vector)) {
+if (unlikely(!msix_enabled(PCI_DEVICE(n)) && vector)) {
trace_pci_nvme_err_invalid_create_cq_vector(vector);
[PULL 2/6] hw/nvme: rename shadow doorbell related trace events
From: Klaus Jensen
Rename the trace events related to writing the event index and reading
the doorbell value to make it more clear that the event is associated
with an actual update (write or read respectively).
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Keith Busch
Signed-off-by: Klaus Jensen
---
hw/nvme/ctrl.c | 11 +++
hw/nvme/trace-events | 8
2 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 78c2f4e39d0a..cfe16476f0a4 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -1337,8 +1337,9 @@ static inline void nvme_blk_write(BlockBackend *blk,
int64_t offset,
static void nvme_update_cq_head(NvmeCQueue *cq)
{
pci_dma_read(PCI_DEVICE(cq->ctrl), cq->db_addr, >head,
-sizeof(cq->head));
-trace_pci_nvme_shadow_doorbell_cq(cq->cqid, cq->head);
+ sizeof(cq->head));
+
+trace_pci_nvme_update_cq_head(cq->cqid, cq->head);
}
static void nvme_post_cqes(void *opaque)
@@ -6147,16 +6148,18 @@ static uint16_t nvme_admin_cmd(NvmeCtrl *n, NvmeRequest
*req)
static void nvme_update_sq_eventidx(const NvmeSQueue *sq)
{
+trace_pci_nvme_update_sq_eventidx(sq->sqid, sq->tail);
+
pci_dma_write(PCI_DEVICE(sq->ctrl), sq->ei_addr, >tail,
sizeof(sq->tail));
-trace_pci_nvme_eventidx_sq(sq->sqid, sq->tail);
}
static void nvme_update_sq_tail(NvmeSQueue *sq)
{
pci_dma_read(PCI_DEVICE(sq->ctrl), sq->db_addr, >tail,
sizeof(sq->tail));
-trace_pci_nvme_shadow_doorbell_sq(sq->sqid, sq->tail);
+
+trace_pci_nvme_update_sq_tail(sq->sqid, sq->tail);
}
static void nvme_process_sq(void *opaque)
diff --git a/hw/nvme/trace-events b/hw/nvme/trace-events
index fccb79f48973..b16f2260b4fd 100644
--- a/hw/nvme/trace-events
+++ b/hw/nvme/trace-events
@@ -84,8 +84,8 @@ pci_nvme_enqueue_event_noqueue(int queued) "queued %d"
pci_nvme_enqueue_event_masked(uint8_t typ) "type 0x%"PRIx8""
pci_nvme_no_outstanding_aers(void) "ignoring event; no outstanding AERs"
pci_nvme_enqueue_req_completion(uint16_t cid, uint16_t cqid, uint32_t dw0,
uint32_t dw1, uint16_t status) "cid %"PRIu16" cqid %"PRIu16" dw0 0x%"PRIx32"
dw1 0x%"PRIx32" status 0x%"PRIx16""
-pci_nvme_eventidx_cq(uint16_t cqid, uint16_t new_eventidx) "cqid %"PRIu16"
new_eventidx %"PRIu16""
-pci_nvme_eventidx_sq(uint16_t sqid, uint16_t new_eventidx) "sqid %"PRIu16"
new_eventidx %"PRIu16""
+pci_nvme_update_cq_eventidx(uint16_t cqid, uint16_t new_eventidx) "cqid
%"PRIu16" new_eventidx %"PRIu16""
+pci_nvme_update_sq_eventidx(uint16_t sqid, uint16_t new_eventidx) "sqid
%"PRIu16" new_eventidx %"PRIu16""
pci_nvme_mmio_read(uint64_t addr, unsigned size) "addr 0x%"PRIx64" size %d"
pci_nvme_mmio_write(uint64_t addr, uint64_t data, unsigned size) "addr
0x%"PRIx64" data 0x%"PRIx64" size %d"
pci_nvme_mmio_doorbell_cq(uint16_t cqid, uint16_t new_head) "cqid %"PRIu16"
new_head %"PRIu16""
@@ -102,8 +102,8 @@ pci_nvme_mmio_start_success(void) "setting controller
enable bit succeeded"
pci_nvme_mmio_stopped(void) "cleared controller enable bit"
pci_nvme_mmio_shutdown_set(void) "shutdown bit set"
pci_nvme_mmio_shutdown_cleared(void) "shutdown bit cleared"
-pci_nvme_shadow_doorbell_cq(uint16_t cqid, uint16_t new_shadow_doorbell) "cqid
%"PRIu16" new_shadow_doorbell %"PRIu16""
-pci_nvme_shadow_doorbell_sq(uint16_t sqid, uint16_t new_shadow_doorbell) "sqid
%"PRIu16" new_shadow_doorbell %"PRIu16""
+pci_nvme_update_cq_head(uint16_t cqid, uint16_t new_head) "cqid %"PRIu16"
new_head %"PRIu16""
+pci_nvme_update_sq_tail(uint16_t sqid, uint16_t new_tail) "sqid %"PRIu16"
new_tail %"PRIu16""
pci_nvme_open_zone(uint64_t slba, uint32_t zone_idx, int all) "open zone,
slba=%"PRIu64", idx=%"PRIu32", all=%"PRIi32""
pci_nvme_close_zone(uint64_t slba, uint32_t zone_idx, int all) "close zone,
slba=%"PRIu64", idx=%"PRIu32", all=%"PRIi32""
pci_nvme_finish_zone(uint64_t slba, uint32_t zone_idx, int all) "finish zone,
slba=%"PRIu64", idx=%"PRIu32", all=%"PRIi32""
--
2.39.0
[PULL 0/6] hw/nvme updates
From: Klaus Jensen Hi Peter, The following changes since commit 528d9f33cad5245c1099d77084c78bb2244d5143: Merge tag 'pull-tcg-20230106' of https://gitlab.com/rth7680/qemu into staging (2023-01-08 11:23:17 +) are available in the Git repository at: https://gitlab.com/birkelund/qemu.git tags/nvme-next-pull-request for you to fetch changes up to 973f76cf7743545a5d8a0a8bfdfe2cd02aa3e238: hw/nvme: cleanup error reporting in nvme_init_pci() (2023-01-11 08:41:19 +0100) hw/nvme updates Klaus Jensen (6): hw/nvme: use QOM accessors hw/nvme: rename shadow doorbell related trace events hw/nvme: fix missing endian conversions for doorbell buffers hw/nvme: fix missing cq eventidx update hw/nvme: clean up confusing use of errp/local_err hw/nvme: cleanup error reporting in nvme_init_pci() hw/nvme/ctrl.c | 194 --- hw/nvme/trace-events | 8 +- 2 files changed, 113 insertions(+), 89 deletions(-) -- 2.39.0
Re: [PATCH] bulk: Rename TARGET_FMT_plx -> HWADDR_FMT_plx
On 10/1/23 23:01, BALATON Zoltan wrote: On Tue, 10 Jan 2023, Philippe Mathieu-Daudé wrote: The 'hwaddr' type is defined in "exec/hwaddr.h" as: hwaddr is the type of a physical address (its size can be different from 'target_ulong'). All definitions use the 'HWADDR_' prefix, except TARGET_FMT_plx: $ fgrep define include/exec/hwaddr.h #define HWADDR_H #define HWADDR_BITS 64 #define HWADDR_MAX UINT64_MAX #define TARGET_FMT_plx "%016" PRIx64 ^^ #define HWADDR_PRId PRId64 #define HWADDR_PRIi PRIi64 #define HWADDR_PRIo PRIo64 #define HWADDR_PRIu PRIu64 #define HWADDR_PRIx PRIx64 Why are there both TARGET_FMT_plx and HWADDR_PRIx? Why not just use HWADDR_PRIx instead? Too lazy to specify the 0-digit alignment format I presume?
Re: [PATCH 0/1] hw/ide: share bmdma read and write functions
Am 9. Januar 2023 19:24:16 UTC schrieb John Snow : >On Tue, Sep 6, 2022 at 10:27 AM Bernhard Beschow wrote: >> >> Am 19. Februar 2022 08:08:17 UTC schrieb Liav Albani : >> >This is a preparation before I send v3 of ich6-ide controller emulation >> >patch. >> >I figured that it's more trivial to split the changes this way, by >> >extracting >> >the bmdma functions from via.c and piix.c and sharing them together. Then, >> >I could easily put these into use when I send v3 of the ich6-ide patch by >> >just >> >using the already separated functions. This was suggested by BALATON Zoltan >> >when >> >he submitted a code review on my ich6-ide controller emulation patch. >> >> Ping. Any news? > >*cough*. > >Has this been folded into subsequent series, or does this still need attention? Both piix and via still have their own bmdma implementations. This patch might be worth having. Best regards, Bernhard > >> >> >Liav Albani (1): >> > hw/ide: share bmdma read and write functions between piix.c and via.c >> > >> > hw/ide/pci.c | 47 >> > hw/ide/piix.c| 50 ++- >> > hw/ide/via.c | 51 ++-- >> > include/hw/ide/pci.h | 4 >> > 4 files changed, 55 insertions(+), 97 deletions(-) >> > >> >
Re: [PATCH] bulk: Rename TARGET_FMT_plx -> HWADDR_FMT_plx
On Tue, 10 Jan 2023, Philippe Mathieu-Daudé wrote: The 'hwaddr' type is defined in "exec/hwaddr.h" as: hwaddr is the type of a physical address (its size can be different from 'target_ulong'). All definitions use the 'HWADDR_' prefix, except TARGET_FMT_plx: $ fgrep define include/exec/hwaddr.h #define HWADDR_H #define HWADDR_BITS 64 #define HWADDR_MAX UINT64_MAX #define TARGET_FMT_plx "%016" PRIx64 ^^ #define HWADDR_PRId PRId64 #define HWADDR_PRIi PRIi64 #define HWADDR_PRIo PRIo64 #define HWADDR_PRIu PRIu64 #define HWADDR_PRIx PRIx64 Why are there both TARGET_FMT_plx and HWADDR_PRIx? Why not just use HWADDR_PRIx instead? Regards, BALATON Zoltan
[PATCH] bulk: Rename TARGET_FMT_plx -> HWADDR_FMT_plx
The 'hwaddr' type is defined in "exec/hwaddr.h" as:
hwaddr is the type of a physical address
(its size can be different from 'target_ulong').
All definitions use the 'HWADDR_' prefix, except TARGET_FMT_plx:
$ fgrep define include/exec/hwaddr.h
#define HWADDR_H
#define HWADDR_BITS 64
#define HWADDR_MAX UINT64_MAX
#define TARGET_FMT_plx "%016" PRIx64
^^
#define HWADDR_PRId PRId64
#define HWADDR_PRIi PRIi64
#define HWADDR_PRIo PRIo64
#define HWADDR_PRIu PRIu64
#define HWADDR_PRIx PRIx64
#define HWADDR_PRIX PRIX64
Since hwaddr's size can be *different* from target_ulong, it is
very confusing to read one of its format using the 'TARGET_FMT_'
prefix, normally used for the target_long / target_ulong types:
$ fgrep TARGET_FMT_ include/exec/cpu-defs.h
#define TARGET_FMT_lx "%08x"
#define TARGET_FMT_ld "%d"
#define TARGET_FMT_lu "%u"
#define TARGET_FMT_lx "%016" PRIx64
#define TARGET_FMT_ld "%" PRId64
#define TARGET_FMT_lu "%" PRIu64
Apparently this format was missed during commit a8170e5e97
("Rename target_phys_addr_t to hwaddr"), so complete it by
doing a bulk-rename with:
$ sed -i -e s/TARGET_FMT_plx/HWADDR_FMT_plx/g $(git grep -l TARGET_FMT_plx)
Signed-off-by: Philippe Mathieu-Daudé
---
accel/tcg/cputlb.c | 2 +-
hw/arm/strongarm.c | 24
hw/block/pflash_cfi01.c | 2 +-
hw/char/digic-uart.c| 4 ++--
hw/char/etraxfs_ser.c | 4 ++--
hw/core/loader.c| 8
hw/core/sysbus.c| 4 ++--
hw/display/cirrus_vga.c | 4 ++--
hw/display/g364fb.c | 4 ++--
hw/display/vga.c| 8
hw/dma/etraxfs_dma.c| 14 +++---
hw/dma/pl330.c | 14 +++---
hw/dma/xilinx_axidma.c | 4 ++--
hw/dma/xlnx_csu_dma.c | 4 ++--
hw/i2c/mpc_i2c.c| 4 ++--
hw/i386/multiboot.c | 8
hw/i386/xen/xen-hvm.c | 8
hw/i386/xen/xen-mapcache.c | 16
hw/i386/xen/xen_platform.c | 4 ++--
hw/intc/arm_gicv3_dist.c| 8
hw/intc/arm_gicv3_its.c | 14 +++---
hw/intc/arm_gicv3_redist.c | 8
hw/intc/exynos4210_combiner.c | 10 +-
hw/misc/auxbus.c| 2 +-
hw/misc/ivshmem.c | 6 +++---
hw/misc/macio/mac_dbdma.c | 4 ++--
hw/misc/mst_fpga.c | 4 ++--
hw/net/allwinner-sun8i-emac.c | 4 ++--
hw/net/allwinner_emac.c | 4 ++--
hw/net/fsl_etsec/etsec.c| 4 ++--
hw/net/fsl_etsec/rings.c| 4 ++--
hw/net/pcnet.c | 4 ++--
hw/net/rocker/rocker.c | 26 +-
hw/net/rocker/rocker_desc.c | 2 +-
hw/net/xilinx_axienet.c | 4 ++--
hw/net/xilinx_ethlite.c | 6 +++---
hw/pci-bridge/pci_expander_bridge.c | 2 +-
hw/pci-host/bonito.c| 14 +++---
hw/pci-host/ppce500.c | 4 ++--
hw/pci/pci_host.c | 4 ++--
hw/ppc/ppc4xx_sdram.c | 2 +-
hw/rtc/exynos4210_rtc.c | 4 ++--
hw/sh4/sh7750.c | 4 ++--
hw/ssi/xilinx_spi.c | 4 ++--
hw/ssi/xilinx_spips.c | 8
hw/timer/digic-timer.c | 4 ++--
hw/timer/etraxfs_timer.c| 2 +-
hw/timer/exynos4210_mct.c | 2 +-
hw/timer/exynos4210_pwm.c | 4 ++--
hw/virtio/virtio-mmio.c | 4 ++--
hw/xen/xen_pt.c | 4 ++--
include/exec/hwaddr.h | 2 +-
monitor/misc.c | 2 +-
softmmu/memory.c| 18 +-
softmmu/memory_mapping.c| 4 ++--
softmmu/physmem.c | 10 +-
target/i386/monitor.c | 6 +++---
target/loongarch/tlb_helper.c | 2 +-
target/microblaze/op_helper.c | 2 +-
target/mips/tcg/sysemu/tlb_helper.c | 2 +-
target/ppc/mmu-hash32.c | 14 +++---
target/ppc/mmu-hash64.c | 12 ++--
target/ppc/mmu_common.c | 26 +-
target/ppc/mmu_helper.c | 4 ++--
target/riscv/cpu_helper.c | 10 +-
target/riscv/monitor.c | 2 +-
target/sparc/ldst_helper.c | 6 +++---
target/sparc/mmu_helper.c | 10 +-
target/tricore/helper.c | 2 +-
69 files changed, 227 insertions(+), 227 deletions(-)
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 4948729917..4e040a1cb9 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1142,7 +1142,7 @@ void tlb_set_page_full(CPUState *cpu, int mmu_idx,
, ,
Re: [PATCH v4 04/11] iotests: QemuStorageDaemon: add cmd() method like in QEMUMachine.
On Tue, Jan 10, 2023 at 3:38 AM Vladimir Sementsov-Ogievskiy wrote: > > Add similar method for consistency. > > Signed-off-by: Vladimir Sementsov-Ogievskiy > --- > tests/qemu-iotests/iotests.py | 4 > 1 file changed, 4 insertions(+) > > diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py > index c69b10ac82..dd08cd8a2b 100644 > --- a/tests/qemu-iotests/iotests.py > +++ b/tests/qemu-iotests/iotests.py > @@ -462,6 +462,10 @@ def qmp(self, cmd: str, args: Optional[Dict[str, > object]] = None) \ > assert self._qmp is not None > return self._qmp.cmd_raw(cmd, args) > > +def cmd(self, cmd: str, args: Optional[Dict[str, object]] = None) \ > +-> QMPMessage: > +return self._qmp.cmd(cmd, **args) > + The typing of this is off -- try "make check-dev" in qemu.git/python to see: iotests.py:467: error: Item "None" of "Optional[QEMUMonitorProtocol]" has no attribute "cmd" [union-attr] iotests.py:467: error: Argument after ** must be a mapping, not "Optional[Dict[str, object]]" [arg-type] iotests.py:467: error: Incompatible return value type (got "Union[object, Any]", expected "Dict[str, Any]") [return-value] Found 3 errors in 1 file (checked 32 source files) You need to assert that self._qmp is not None for the first; the second seems to do with a potentially "None" argument for args, and the third has to do with the difference between returning the entire raw response and just the return value. I started making a fixup branch, but I stopped around here. https://gitlab.com/jsnow/qemu/-/commits/vlad-iotest-patches > def stop(self, kill_signal=15): > self._p.send_signal(kill_signal) > self._p.wait() > -- > 2.34.1 >
Re: [PATCH] python: QEMUMachine: enable qmp accept timeout by default
On Tue, Jan 10, 2023, 3:53 AM Vladimir Sementsov-Ogievskiy < vsement...@yandex-team.ru> wrote: > On 7/12/22 00:21, John Snow wrote: > > On Mon, Jul 11, 2022 at 5:16 PM John Snow wrote: > >> > >> On Fri, Jun 24, 2022 at 3:53 PM Vladimir Sementsov-Ogievskiy > >> wrote: > >>> > >>> I've spent much time trying to debug hanging pipeline in gitlab. I > >>> started from and idea that I have problem in code in my series (which > >>> has some timeouts). Finally I found that the problem is that I've used > >>> QEMUMachine class directly to avoid qtest, and didn't add necessary > >>> arguments. Qemu fails and we wait for qmp accept endlessly. In gitlab > >>> it's just stopped by timeout (one hour) with no sign of what's going > >>> wrong. > >>> > >>> With timeout enabled, gitlab don't wait for an hour and prints all > >>> needed information. > >>> > >>> Signed-off-by: Vladimir Sementsov-Ogievskiy > > >>> --- > >>> > >>> Hi all! > >>> > >>> Just compare this > >>>https://gitlab.com/vsementsov/qemu/-/pipelines/572232557 > >>> and this > >>>https://gitlab.com/vsementsov/qemu/-/pipelines/572526252 > >>> > >>> and you'll see that the latter is much better. > >>> > >>> python/qemu/machine/machine.py | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/python/qemu/machine/machine.py > b/python/qemu/machine/machine.py > >>> index 37191f433b..01a12f6f73 100644 > >>> --- a/python/qemu/machine/machine.py > >>> +++ b/python/qemu/machine/machine.py > >>> @@ -131,7 +131,7 @@ def __init__(self, > >>>drain_console: bool = False, > >>>console_log: Optional[str] = None, > >>>log_dir: Optional[str] = None, > >>> - qmp_timer: Optional[float] = None): > >>> + qmp_timer: float = 30): > >>> ''' > >>> Initialize a QEMUMachine > >>> > >>> -- > >>> 2.25.1 > >>> > >> > >> Oh, this is because machine.py uses the qmp_timer for *all* timeouts, > >> and not just the QMP commands themselves, and this relates to the work > >> Marc Andre is doing with regards to changing the launch mechanism to > >> handle the race condition when the QEMU launch fails, but the QMP > >> connection just sits waiting. > >> > >> I'm quite of the mind that it's really time to rewrite machine.py to > >> use the native asyncio interfaces I've been writing to help manage > >> this, but in the meantime I think this is probably a reasonable > >> concession and a more useful default. > >> > >> ...I think. Willing to take it for now and re-investigate when the > >> other fixes make it to the tree. > >> > >> Reviewed-by: John Snow > > > > Oh, keep the type as Optional[float], though, so the timeout can be > > disabled again, and keeps the type consistent with the qtest > > derivative class. I've staged your patch with that change made, let me > > know if that's not OK. Modified patch is on my python branch: > > > > Thanks, merged. > > > > Hmm, seems that's lost.. I don't see it neither in master nor in your > python branch.. > > -- > Best regards, > Vladimir > :( I'll fix it. Thanks for resending the iotests series, too - the old version was at the very top of my inbox :) >
Re: [PATCH] python: QEMUMachine: enable qmp accept timeout by default
On Tue, Jan 10, 2023 at 12:06 PM John Snow wrote: > > > > On Tue, Jan 10, 2023, 3:53 AM Vladimir Sementsov-Ogievskiy > wrote: >> >> On 7/12/22 00:21, John Snow wrote: >> > On Mon, Jul 11, 2022 at 5:16 PM John Snow wrote: >> >> >> >> On Fri, Jun 24, 2022 at 3:53 PM Vladimir Sementsov-Ogievskiy >> >> wrote: >> >>> >> >>> I've spent much time trying to debug hanging pipeline in gitlab. I >> >>> started from and idea that I have problem in code in my series (which >> >>> has some timeouts). Finally I found that the problem is that I've used >> >>> QEMUMachine class directly to avoid qtest, and didn't add necessary >> >>> arguments. Qemu fails and we wait for qmp accept endlessly. In gitlab >> >>> it's just stopped by timeout (one hour) with no sign of what's going >> >>> wrong. >> >>> >> >>> With timeout enabled, gitlab don't wait for an hour and prints all >> >>> needed information. >> >>> >> >>> Signed-off-by: Vladimir Sementsov-Ogievskiy >> >>> --- >> >>> >> >>> Hi all! >> >>> >> >>> Just compare this >> >>>https://gitlab.com/vsementsov/qemu/-/pipelines/572232557 >> >>> and this >> >>>https://gitlab.com/vsementsov/qemu/-/pipelines/572526252 >> >>> >> >>> and you'll see that the latter is much better. >> >>> >> >>> python/qemu/machine/machine.py | 2 +- >> >>> 1 file changed, 1 insertion(+), 1 deletion(-) >> >>> >> >>> diff --git a/python/qemu/machine/machine.py >> >>> b/python/qemu/machine/machine.py >> >>> index 37191f433b..01a12f6f73 100644 >> >>> --- a/python/qemu/machine/machine.py >> >>> +++ b/python/qemu/machine/machine.py >> >>> @@ -131,7 +131,7 @@ def __init__(self, >> >>>drain_console: bool = False, >> >>>console_log: Optional[str] = None, >> >>>log_dir: Optional[str] = None, >> >>> - qmp_timer: Optional[float] = None): >> >>> + qmp_timer: float = 30): >> >>> ''' >> >>> Initialize a QEMUMachine >> >>> >> >>> -- >> >>> 2.25.1 >> >>> >> >> >> >> Oh, this is because machine.py uses the qmp_timer for *all* timeouts, >> >> and not just the QMP commands themselves, and this relates to the work >> >> Marc Andre is doing with regards to changing the launch mechanism to >> >> handle the race condition when the QEMU launch fails, but the QMP >> >> connection just sits waiting. >> >> >> >> I'm quite of the mind that it's really time to rewrite machine.py to >> >> use the native asyncio interfaces I've been writing to help manage >> >> this, but in the meantime I think this is probably a reasonable >> >> concession and a more useful default. >> >> >> >> ...I think. Willing to take it for now and re-investigate when the >> >> other fixes make it to the tree. >> >> >> >> Reviewed-by: John Snow >> > >> > Oh, keep the type as Optional[float], though, so the timeout can be >> > disabled again, and keeps the type consistent with the qtest >> > derivative class. I've staged your patch with that change made, let me >> > know if that's not OK. Modified patch is on my python branch: >> > >> > Thanks, merged. >> > >> >> Hmm, seems that's lost.. I don't see it neither in master nor in your python >> branch.. >> >> -- >> Best regards, >> Vladimir > > > :( > > I'll fix it. Thanks for resending the iotests series, too - the old version > was at the very top of my inbox :) Re-edited and Re-staged: https://gitlab.com/jsnow/qemu/-/commits/python --js
Re: [PATCH v5 10/14] vfio/migration: Implement VFIO migration protocol v2
Hello Avihai
On 1/10/23 15:08, Avihai Horon wrote:
On 09/01/2023 20:36, Jason Gunthorpe wrote:
On Mon, Jan 09, 2023 at 06:27:21PM +0100, Cédric Le Goater wrote:
also, in vfio_migration_query_flags() :
+static int vfio_migration_query_flags(VFIODevice *vbasedev, uint64_t
*mig_flags)
+{
+ uint64_t buf[DIV_ROUND_UP(sizeof(struct vfio_device_feature) +
+ sizeof(struct
vfio_device_feature_migration),
+ sizeof(uint64_t))] = {};
+ struct vfio_device_feature *feature = (struct vfio_device_feature *)buf;
+ struct vfio_device_feature_migration *mig =
+ (struct vfio_device_feature_migration *)feature->data;
+
+ feature->argsz = sizeof(buf);
+ feature->flags = VFIO_DEVICE_FEATURE_GET |
VFIO_DEVICE_FEATURE_MIGRATION;
+ if (ioctl(vbasedev->fd, VFIO_DEVICE_FEATURE, feature)) {
+ return -EOPNOTSUPP;
+ }
+
+ *mig_flags = mig->flags;
+
+ return 0;
+}
The code is using any possible error returned by the VFIO_DEVICE_FEATURE
ioctl to distinguish protocol v1 from v2.
I'm comfortable with that from a kernel perspective.
There is no such thing as this API failing in the kernel but userspace
should continue on, no matter what the error code. So always failing
here is correct.
About the only thing you might want to do is convert anything other
than ENOTTY into a hard qemu failure similar to failing to open
/dev/vfio or something - it means something has gone really
wrong.. But that is pretty obscure stuff
Hi Cedric,
With Jason's input, is it ok by you to leave the code as is?
The patchset removes v1 later on, I think we are fine. I was reading it
sequentially and it felt like a weak spot.
All errors are translated in EOPNOTSUPP. That's always true for pre-v6.0
kernels, returning -ENOTTY, and v6.0+ kernels will do the same unless a
mlx5vf device is passthru. I still wonder if we should report some errors
for the ! -ENOTTY case. So the code below could be a good addition.
Thanks,
C.
if not, would this be fine?
+static int vfio_migration_query_flags(VFIODevice *vbasedev, uint64_t
*mig_flags)
+{
+ uint64_t buf[DIV_ROUND_UP(sizeof(struct vfio_device_feature) +
+ sizeof(struct vfio_device_feature_migration),
+ sizeof(uint64_t))] = {};
+ struct vfio_device_feature *feature = (struct vfio_device_feature *)buf;
+ struct vfio_device_feature_migration *mig =
+ (struct vfio_device_feature_migration *)feature->data;
+
+ feature->argsz = sizeof(buf);
+ feature->flags = VFIO_DEVICE_FEATURE_GET | VFIO_DEVICE_FEATURE_MIGRATION;
+ if (ioctl(vbasedev->fd, VFIO_DEVICE_FEATURE, feature)) {
+ if (errno == ENOTTY) {
+ error_report("%s: VFIO migration is not supported in kernel",
+ vbasedev->name);
+ } else {
+ error_report("%s: Failed to query VFIO migration support, err: %s",
+ vbasedev->name, strerror(errno));
+ }
+
+ return -errno;
+ }
+
+ *mig_flags = mig->flags;
+
+ return 0;
+}
+
and then in vfio_migration_init() prior v1 removal:
+ ret = vfio_migration_query_flags(vbasedev, _flags);
+ if (!ret) {
+ /* Migration v2 */
+ } else if (ret == -ENOTTY) {
+ /* Migration v1 */
+ } else {
+ return ret;
+ }
and after v1 removal vfio_migration_init() will be:
ret = vfio_migration_query_flags(vbasedev, _flags);
if (ret) {
return ret;
}
Thanks.
Zoned storage support in libvirt
Hi Peter, Zoned storage support (https://zonedstorage.io/docs/introduction/zoned-storage) is being added to QEMU. Given a zoned host block device, the QEMU syntax will look like this: --blockdev zoned_host_device,node-name=drive0,filename=/dev/$BDEV,... --device virtio-blk-pci,drive=drive0 Note that regular --blockdev host_device will not work. For now the virtio-blk device is the only one that supports zoned blockdevs. This brings to mind a few questions: 1. Does libvirt need domain XML syntax for zoned storage? Alternatively, it could probe /sys/block/$BDEV/queue/zoned and generate the correct QEMU command-line arguments for zoned devices when the contents of the file are not "none". 2. Should QEMU --blockdev host_device detected zoned devices so that --blockdev zoned_host_device is not necessary? That way libvirt would automatically support zoned storage without any domain XML syntax or libvirt code changes. The drawbacks I see when QEMU detects zoned storage automatically: - You can't easiy tell if a blockdev is zoned from the command-line. - It's possible to mismatch zoned and non-zoned devices across live migration. We still have time to decide on the QEMU command-line syntax for QEMU 8.0, so I wanted to raise this now. Thanks, Stefan signature.asc Description: PGP signature
Re: [PULL 0/4] hw/nvme updates
On Tue, 10 Jan 2023 at 07:20, Klaus Jensen wrote: > > On Jan 10 08:17, Klaus Jensen wrote: > > From: Klaus Jensen > > > > Hi, > > > > The following changes since commit 528d9f33cad5245c1099d77084c78bb2244d5143: > > > > Merge tag 'pull-tcg-20230106' of https://gitlab.com/rth7680/qemu into > > staging (2023-01-08 11:23:17 +) > > > > are available in the Git repository at: > > > > git://git.infradead.org/qemu-nvme.git tags/nvme-next-pull-request > > > > for you to fetch changes up to fa5db2aa168bdc0f15c269b6212ef47632fab8ba: > > > > hw/nvme: fix missing cq eventidx update (2023-01-09 08:48:46 +0100) > > > > > > hw/nvme updates > > > > > > > > Klaus Jensen (4): > > hw/nvme: use QOM accessors > > hw/nvme: rename shadow doorbell related trace events > > hw/nvme: fix missing endian conversions for doorbell buffers > > hw/nvme: fix missing cq eventidx update > > > > hw/nvme/ctrl.c | 121 ++- > > hw/nvme/trace-events | 8 +-- > > 2 files changed, 78 insertions(+), 51 deletions(-) > > > > -- > > 2.39.0 > > > > Argh. I forgot to update the pull url to something https://. > > Do I need to send a new pull? You can just send a new cover letter with the right URL-and-tag-line (I tried guessing what the right URL was, but didn't guess right.) thanks -- PMM
Re: Zoned storage support in libvirt
On Tue, Jan 10, 2023 at 10:19:51AM -0500, Stefan Hajnoczi wrote: > Hi Peter, > Zoned storage support > (https://zonedstorage.io/docs/introduction/zoned-storage) is being added > to QEMU. Given a zoned host block device, the QEMU syntax will look like > this: > > --blockdev zoned_host_device,node-name=drive0,filename=/dev/$BDEV,... > --device virtio-blk-pci,drive=drive0 > > Note that regular --blockdev host_device will not work. > > For now the virtio-blk device is the only one that supports zoned > blockdevs. Does the virtio-blk device expowsed guest ABI differ at all when connected zoned_host_device instead of host_device ? > This brings to mind a few questions: > > 1. Does libvirt need domain XML syntax for zoned storage? Alternatively, >it could probe /sys/block/$BDEV/queue/zoned and generate the correct >QEMU command-line arguments for zoned devices when the contents of >the file are not "none". > > 2. Should QEMU --blockdev host_device detected zoned devices so that >--blockdev zoned_host_device is not necessary? That way libvirt would >automatically support zoned storage without any domain XML syntax or >libvirt code changes. > >The drawbacks I see when QEMU detects zoned storage automatically: >- You can't easiy tell if a blockdev is zoned from the command-line. >- It's possible to mismatch zoned and non-zoned devices across live > migration. What happens with existing QEMU impls if you use --blockdev host_device pointing to a /dev/$BDEV that is a zoned device ? If it succeeds and works correctly, then we likely need to continue to support that. This would push towards needing a new XML element. With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH] tests/qemu-iotests/312: Mark "quorum" as required driver
Am 04.01.2023 um 12:46 hat Thomas Huth geschrieben: > "quorum" is required by iotest 312 - if it is not compiled into the > QEMU binary, the test fails. Thus list "quorum" as required driver > so that the test gets skipped in case it is not available. > > Signed-off-by: Thomas Huth Thanks, applied to the block branch. Kevin
Re: [PATCH] tests/qemu-iotests/262: Check for availability of "blkverify" first
Am 04.01.2023 um 12:28 hat Thomas Huth geschrieben: > In downstream RHEL builds, we do not have "blkverify" enabled, so > iotest 262 is currently failing there. Thus let's list "blkverify" > as required item so that the test properly gets skipped instead if > "blkverify" is missing. > > Signed-off-by: Thomas Huth Thanks, applied to the block branch. Kevin
Re: [PATCH v5 10/14] vfio/migration: Implement VFIO migration protocol v2
On 09/01/2023 20:36, Jason Gunthorpe wrote:
On Mon, Jan 09, 2023 at 06:27:21PM +0100, Cédric Le Goater wrote:
also, in vfio_migration_query_flags() :
+static int vfio_migration_query_flags(VFIODevice *vbasedev, uint64_t
*mig_flags)
+{
+uint64_t buf[DIV_ROUND_UP(sizeof(struct vfio_device_feature) +
+ sizeof(struct
vfio_device_feature_migration),
+ sizeof(uint64_t))] = {};
+struct vfio_device_feature *feature = (struct vfio_device_feature *)buf;
+struct vfio_device_feature_migration *mig =
+(struct vfio_device_feature_migration *)feature->data;
+
+feature->argsz = sizeof(buf);
+feature->flags = VFIO_DEVICE_FEATURE_GET |
VFIO_DEVICE_FEATURE_MIGRATION;
+if (ioctl(vbasedev->fd, VFIO_DEVICE_FEATURE, feature)) {
+return -EOPNOTSUPP;
+}
+
+*mig_flags = mig->flags;
+
+return 0;
+}
The code is using any possible error returned by the VFIO_DEVICE_FEATURE
ioctl to distinguish protocol v1 from v2.
I'm comfortable with that from a kernel perspective.
There is no such thing as this API failing in the kernel but userspace
should continue on, no matter what the error code. So always failing
here is correct.
About the only thing you might want to do is convert anything other
than ENOTTY into a hard qemu failure similar to failing to open
/dev/vfio or something - it means something has gone really
wrong.. But that is pretty obscure stuff
Hi Cedric,
With Jason's input, is it ok by you to leave the code as is?
if not, would this be fine?
+static int vfio_migration_query_flags(VFIODevice *vbasedev, uint64_t
*mig_flags)
+{
+ uint64_t buf[DIV_ROUND_UP(sizeof(struct vfio_device_feature) +
+ sizeof(struct
vfio_device_feature_migration),
+ sizeof(uint64_t))] = {};
+ struct vfio_device_feature *feature = (struct vfio_device_feature
*)buf;
+ struct vfio_device_feature_migration *mig =
+ (struct vfio_device_feature_migration *)feature->data;
+
+ feature->argsz = sizeof(buf);
+ feature->flags = VFIO_DEVICE_FEATURE_GET |
VFIO_DEVICE_FEATURE_MIGRATION;
+ if (ioctl(vbasedev->fd, VFIO_DEVICE_FEATURE, feature)) {
+ if (errno == ENOTTY) {
+ error_report("%s: VFIO migration is not supported in kernel",
+ vbasedev->name);
+ } else {
+ error_report("%s: Failed to query VFIO migration support,
err: %s",
+ vbasedev->name, strerror(errno));
+ }
+
+ return -errno;
+ }
+
+ *mig_flags = mig->flags;
+
+ return 0;
+}
+
and then in vfio_migration_init() prior v1 removal:
+ ret = vfio_migration_query_flags(vbasedev, _flags);
+ if (!ret) {
+ /* Migration v2 */
+ } else if (ret == -ENOTTY) {
+ /* Migration v1 */
+ } else {
+ return ret;
+ }
and after v1 removal vfio_migration_init() will be:
ret = vfio_migration_query_flags(vbasedev, _flags);
if (ret) {
return ret;
}
Thanks.
Re: [PATCH v2] pflash: Only read non-zero parts of backend image
Am 20.12.2022 um 09:42 hat Gerd Hoffmann geschrieben: > From: Xiang Zheng > > Currently we fill the VIRT_FLASH memory space with two 64MB NOR images > when using persistent UEFI variables on virt board. Actually we only use > a very small(non-zero) part of the memory while the rest significant > large(zero) part of memory is wasted. > > So this patch checks the block status and only writes the non-zero part > into memory. This requires pflash devices to use sparse files for > backends. > > Signed-off-by: Xiang Zheng > > [ kraxel: rebased to latest master ] > > Signed-off-by: Gerd Hoffmann Thanks, applied to the block branch. Even though discussion is ongoing about using alternative devices, it seems to me that this is a simple optimisation that doesn't change the behaviour as seen by the guest and that we want to have either way. If anyone objects and wants me to drop the patch again, let me know. Kevin
Re: [RFC PATCH 0/4] qom: Introduce object_class_property_deprecate()
Am 09.01.2023 um 23:54 hat Philippe Mathieu-Daudé geschrieben: > Hi, > > There will always be a need to deprecate things. Here I'm > tackling the QOM (class) properties, since they can be set > from some CLI options (-object -device -global ...). > > As an experiment, we add object_class_property_deprecate() > to register a class property as deprecated (since some version), > then we deprecate the TYPE_PFLASH_CFI02 'width' property, and > finally as a bonus we emit a warning when the deprecation period > is over, as a reminder. (For that we introduce few 'versions' > helpers). The last part means that increasing the version number (i.e. the commit that opens the development tree for the next release) can change the output, and this is turn can break test cases. If we are happy to introduce breakage with a version number change that will require future commits to open the development tree less trivial than they are today because they need to fix the breakage, too, why not make it a build error instead of a different warning message at runtime? Kevin
Re: [PATCH v4 01/11] python: rename QEMUMonitorProtocol.cmd() to cmd_raw()
On 1/10/23 13:49, Daniel P. Berrangé wrote:
On Tue, Jan 10, 2023 at 11:37:48AM +0300, Vladimir Sementsov-Ogievskiy wrote:
Having cmd() and command() methods in one class doesn't look good.
Rename cmd() to cmd_raw(), to show its meaning better.
We also want to rename command() to cmd() in future, so this commit is a
necessary first step.
Keep new cmd_raw() only in a few places where it's really needed and
move to command() where possible.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
python/qemu/machine/machine.py | 2 +-
python/qemu/qmp/legacy.py | 8 ++--
python/qemu/qmp/qmp_shell.py | 13 +++--
scripts/cpu-x86-uarch-abi.py | 12 ++--
tests/qemu-iotests/iotests.py | 2 +-
5 files changed, 17 insertions(+), 20 deletions(-)
diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index 748a0d807c..9059dc3948 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -674,7 +674,7 @@ def qmp(self, cmd: str,
conv_keys = True
qmp_args = self._qmp_args(conv_keys, args)
-ret = self._qmp.cmd(cmd, args=qmp_args)
+ret = self._qmp.cmd_raw(cmd, args=qmp_args)
if cmd == 'quit' and 'error' not in ret and 'return' in ret:
self._quit_issued = True
return ret
diff --git a/python/qemu/qmp/legacy.py b/python/qemu/qmp/legacy.py
index 1951754455..8e1a504052 100644
--- a/python/qemu/qmp/legacy.py
+++ b/python/qemu/qmp/legacy.py
@@ -186,21 +186,17 @@ def cmd_obj(self, qmp_cmd: QMPMessage) -> QMPMessage:
)
)
-def cmd(self, name: str,
-args: Optional[Dict[str, object]] = None,
-cmd_id: Optional[object] = None) -> QMPMessage:
+def cmd_raw(self, name: str,
+args: Optional[Dict[str, object]] = None) -> QMPMessage:
"""
Build a QMP command and send it to the QMP Monitor.
:param name: command name (string)
:param args: command arguments (dict)
-:param cmd_id: command id (dict, list, string or int)
"""
qmp_cmd: QMPMessage = {'execute': name}
if args:
qmp_cmd['arguments'] = args
-if cmd_id:
-qmp_cmd['id'] = cmd_id
The commit message didn't say anything about dropping the
cmd_id parameter. Presumably you've found that it is not
used in any caller that exists, but still it feels like
a valid parameter to want to support in this method ?
Hmm, right. That should be a separate patch. Still, I think, when trying to
unify similar functions, it's good to drop first any unused thing, to produce
more clear new interface.
return self.cmd_obj(qmp_cmd)
def command(self, cmd: str, **kwds: object) -> QMPReturnValue:
diff --git a/python/qemu/qmp/qmp_shell.py b/python/qemu/qmp/qmp_shell.py
index 619ab42ced..5c0d87a0ec 100644
--- a/python/qemu/qmp/qmp_shell.py
+++ b/python/qemu/qmp/qmp_shell.py
@@ -98,7 +98,7 @@
Sequence,
)
-from qemu.qmp import ConnectError, QMPError, SocketAddrT
+from qemu.qmp import ConnectError, QMPError, SocketAddrT, ExecuteError
from qemu.qmp.legacy import (
QEMUMonitorProtocol,
QMPBadPortError,
@@ -194,11 +194,12 @@ def close(self) -> None:
super().close()
def _fill_completion(self) -> None:
-cmds = self.cmd('query-commands')
-if 'error' in cmds:
-return
-for cmd in cmds['return']:
-self._completer.append(cmd['name'])
+try:
+cmds = self.command('query-commands')
+for cmd in cmds:
+self._completer.append(cmd['name'])
+except ExecuteError:
+pass
def _completer_setup(self) -> None:
self._completer = QMPCompleter()
I'd suggest that re-writing callers to use 'command' is better
done in a prior patch, so that this patch is purely a rename of
cmd -> cmd_raw with no functional changes intermixed.
Agree, will do. Thanks for reviewing!
diff --git a/scripts/cpu-x86-uarch-abi.py b/scripts/cpu-x86-uarch-abi.py
index 82ff07582f..893afd1b35 100644
--- a/scripts/cpu-x86-uarch-abi.py
+++ b/scripts/cpu-x86-uarch-abi.py
@@ -69,7 +69,7 @@
shell = QEMUMonitorProtocol(sock)
shell.connect()
-models = shell.cmd("query-cpu-definitions")
+models = shell.command("query-cpu-definitions")
# These QMP props don't correspond to CPUID fatures
# so ignore them
@@ -85,7 +85,7 @@
names = []
-for model in models["return"]:
+for model in models:
if "alias-of" in model:
continue
names.append(model["name"])
@@ -93,12 +93,12 @@
models = {}
for name in sorted(names):
-cpu = shell.cmd("query-cpu-model-expansion",
- { "type": "static",
- "model": { "name": name }})
+cpu = shell.command("query-cpu-model-expansion",
+{ "type": "static",
+ "model": {
Re: [PATCH 1/1] qemu-iotests/stream-under-throttle: do not shutdown QEMU
Am 07.12.2022 um 14:14 hat Christian Borntraeger geschrieben: > Without a kernel or boot disk a QEMU on s390 will exit (usually with a > disabled wait state). This breaks the stream-under-throttle test case. > Do not exit qemu if on s390. > > Signed-off-by: Christian Borntraeger Thanks, applied to the block branch. Kevin
Re: [RFC PATCH 2/4] hw/block: Rename TYPE_PFLASH_CFI02 'width' property as 'device-width'
On Mon, Jan 09, 2023 at 11:54:17PM +0100, Philippe Mathieu-Daudé wrote: > Use the same property name than the TYPE_PFLASH_CFI01 model. > > Deprecate the current 'width' property and add the 'device-width' > property pointing to the same field in PFlashCFI02. > > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/block/pflash_cfi02.c | 8 +++- > 1 file changed, 7 insertions(+), 1 deletion(-) docs/about/deprecated.rst must be updated for any deprecations to be valid. With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v4 02/11] python/qemu: rename command() to cmd()
On Tue, Jan 10, 2023 at 11:37:49AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Use a shorter name. We are going to move in iotests from qmp() to
> command() where possible. But command() is longer than qmp() and don't
> look better. Let's rename.
>
> You can simply grep for '\.command(' and for 'def command(' to check
> that everything is updated (command() in tests/docker/docker.py is
> unrelated).
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> docs/devel/testing.rst| 10 +-
> python/qemu/machine/machine.py| 8 +-
> python/qemu/qmp/legacy.py | 2 +-
> python/qemu/qmp/qmp_shell.py | 2 +-
> python/qemu/utils/qemu_ga_client.py | 2 +-
> python/qemu/utils/qom.py | 8 +-
> python/qemu/utils/qom_common.py | 2 +-
> python/qemu/utils/qom_fuse.py | 6 +-
> scripts/cpu-x86-uarch-abi.py | 8 +-
> scripts/device-crash-test | 8 +-
> scripts/render_block_graph.py | 8 +-
> tests/avocado/avocado_qemu/__init__.py| 4 +-
> tests/avocado/cpu_queries.py | 5 +-
> tests/avocado/hotplug_cpu.py | 10 +-
> tests/avocado/info_usernet.py | 4 +-
> tests/avocado/machine_arm_integratorcp.py | 6 +-
> tests/avocado/machine_m68k_nextcube.py| 4 +-
> tests/avocado/machine_mips_malta.py | 6 +-
> tests/avocado/machine_s390_ccw_virtio.py | 28 ++--
> tests/avocado/migration.py| 10 +-
> tests/avocado/pc_cpu_hotplug_props.py | 2 +-
> tests/avocado/version.py | 4 +-
> tests/avocado/virtio_check_params.py | 6 +-
> tests/avocado/virtio_version.py | 5 +-
> tests/avocado/x86_cpu_model_versions.py | 13 +-
> tests/migration/guestperf/engine.py | 150 +++---
> tests/qemu-iotests/256| 34 ++---
> tests/qemu-iotests/257| 36 +++---
> 28 files changed, 198 insertions(+), 193 deletions(-)
Reviewed-by: Daniel P. Berrangé
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH] python: QEMUMachine: enable qmp accept timeout by default
On 7/12/22 00:21, John Snow wrote: On Mon, Jul 11, 2022 at 5:16 PM John Snow wrote: On Fri, Jun 24, 2022 at 3:53 PM Vladimir Sementsov-Ogievskiy wrote: I've spent much time trying to debug hanging pipeline in gitlab. I started from and idea that I have problem in code in my series (which has some timeouts). Finally I found that the problem is that I've used QEMUMachine class directly to avoid qtest, and didn't add necessary arguments. Qemu fails and we wait for qmp accept endlessly. In gitlab it's just stopped by timeout (one hour) with no sign of what's going wrong. With timeout enabled, gitlab don't wait for an hour and prints all needed information. Signed-off-by: Vladimir Sementsov-Ogievskiy --- Hi all! Just compare this https://gitlab.com/vsementsov/qemu/-/pipelines/572232557 and this https://gitlab.com/vsementsov/qemu/-/pipelines/572526252 and you'll see that the latter is much better. python/qemu/machine/machine.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py index 37191f433b..01a12f6f73 100644 --- a/python/qemu/machine/machine.py +++ b/python/qemu/machine/machine.py @@ -131,7 +131,7 @@ def __init__(self, drain_console: bool = False, console_log: Optional[str] = None, log_dir: Optional[str] = None, - qmp_timer: Optional[float] = None): + qmp_timer: float = 30): ''' Initialize a QEMUMachine -- 2.25.1 Oh, this is because machine.py uses the qmp_timer for *all* timeouts, and not just the QMP commands themselves, and this relates to the work Marc Andre is doing with regards to changing the launch mechanism to handle the race condition when the QEMU launch fails, but the QMP connection just sits waiting. I'm quite of the mind that it's really time to rewrite machine.py to use the native asyncio interfaces I've been writing to help manage this, but in the meantime I think this is probably a reasonable concession and a more useful default. ...I think. Willing to take it for now and re-investigate when the other fixes make it to the tree. Reviewed-by: John Snow Oh, keep the type as Optional[float], though, so the timeout can be disabled again, and keeps the type consistent with the qtest derivative class. I've staged your patch with that change made, let me know if that's not OK. Modified patch is on my python branch: Thanks, merged. Hmm, seems that's lost.. I don't see it neither in master nor in your python branch.. -- Best regards, Vladimir
Re: [PATCH v4 01/11] python: rename QEMUMonitorProtocol.cmd() to cmd_raw()
On Tue, Jan 10, 2023 at 11:37:48AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Having cmd() and command() methods in one class doesn't look good.
> Rename cmd() to cmd_raw(), to show its meaning better.
>
> We also want to rename command() to cmd() in future, so this commit is a
> necessary first step.
>
> Keep new cmd_raw() only in a few places where it's really needed and
> move to command() where possible.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> python/qemu/machine/machine.py | 2 +-
> python/qemu/qmp/legacy.py | 8 ++--
> python/qemu/qmp/qmp_shell.py | 13 +++--
> scripts/cpu-x86-uarch-abi.py | 12 ++--
> tests/qemu-iotests/iotests.py | 2 +-
> 5 files changed, 17 insertions(+), 20 deletions(-)
>
> diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
> index 748a0d807c..9059dc3948 100644
> --- a/python/qemu/machine/machine.py
> +++ b/python/qemu/machine/machine.py
> @@ -674,7 +674,7 @@ def qmp(self, cmd: str,
> conv_keys = True
>
> qmp_args = self._qmp_args(conv_keys, args)
> -ret = self._qmp.cmd(cmd, args=qmp_args)
> +ret = self._qmp.cmd_raw(cmd, args=qmp_args)
> if cmd == 'quit' and 'error' not in ret and 'return' in ret:
> self._quit_issued = True
> return ret
> diff --git a/python/qemu/qmp/legacy.py b/python/qemu/qmp/legacy.py
> index 1951754455..8e1a504052 100644
> --- a/python/qemu/qmp/legacy.py
> +++ b/python/qemu/qmp/legacy.py
> @@ -186,21 +186,17 @@ def cmd_obj(self, qmp_cmd: QMPMessage) -> QMPMessage:
> )
> )
>
> -def cmd(self, name: str,
> -args: Optional[Dict[str, object]] = None,
> -cmd_id: Optional[object] = None) -> QMPMessage:
> +def cmd_raw(self, name: str,
> +args: Optional[Dict[str, object]] = None) -> QMPMessage:
> """
> Build a QMP command and send it to the QMP Monitor.
>
> :param name: command name (string)
> :param args: command arguments (dict)
> -:param cmd_id: command id (dict, list, string or int)
> """
> qmp_cmd: QMPMessage = {'execute': name}
> if args:
> qmp_cmd['arguments'] = args
> -if cmd_id:
> -qmp_cmd['id'] = cmd_id
The commit message didn't say anything about dropping the
cmd_id parameter. Presumably you've found that it is not
used in any caller that exists, but still it feels like
a valid parameter to want to support in this method ?
> return self.cmd_obj(qmp_cmd)
>
> def command(self, cmd: str, **kwds: object) -> QMPReturnValue:
> diff --git a/python/qemu/qmp/qmp_shell.py b/python/qemu/qmp/qmp_shell.py
> index 619ab42ced..5c0d87a0ec 100644
> --- a/python/qemu/qmp/qmp_shell.py
> +++ b/python/qemu/qmp/qmp_shell.py
> @@ -98,7 +98,7 @@
> Sequence,
> )
>
> -from qemu.qmp import ConnectError, QMPError, SocketAddrT
> +from qemu.qmp import ConnectError, QMPError, SocketAddrT, ExecuteError
> from qemu.qmp.legacy import (
> QEMUMonitorProtocol,
> QMPBadPortError,
> @@ -194,11 +194,12 @@ def close(self) -> None:
> super().close()
>
> def _fill_completion(self) -> None:
> -cmds = self.cmd('query-commands')
> -if 'error' in cmds:
> -return
> -for cmd in cmds['return']:
> -self._completer.append(cmd['name'])
> +try:
> +cmds = self.command('query-commands')
> +for cmd in cmds:
> +self._completer.append(cmd['name'])
> +except ExecuteError:
> +pass
>
> def _completer_setup(self) -> None:
> self._completer = QMPCompleter()
I'd suggest that re-writing callers to use 'command' is better
done in a prior patch, so that this patch is purely a rename of
cmd -> cmd_raw with no functional changes intermixed.
> diff --git a/scripts/cpu-x86-uarch-abi.py b/scripts/cpu-x86-uarch-abi.py
> index 82ff07582f..893afd1b35 100644
> --- a/scripts/cpu-x86-uarch-abi.py
> +++ b/scripts/cpu-x86-uarch-abi.py
> @@ -69,7 +69,7 @@
> shell = QEMUMonitorProtocol(sock)
> shell.connect()
>
> -models = shell.cmd("query-cpu-definitions")
> +models = shell.command("query-cpu-definitions")
>
> # These QMP props don't correspond to CPUID fatures
> # so ignore them
> @@ -85,7 +85,7 @@
>
> names = []
>
> -for model in models["return"]:
> +for model in models:
> if "alias-of" in model:
> continue
> names.append(model["name"])
> @@ -93,12 +93,12 @@
> models = {}
>
> for name in sorted(names):
> -cpu = shell.cmd("query-cpu-model-expansion",
> - { "type": "static",
> - "model": { "name": name }})
> +cpu = shell.command("query-cpu-model-expansion",
> +{ "type": "static",
> + "model": { "name": name }})
>
> got = {}
> -for (feature, present) in
Re: [RFC PATCH 1/4] qom: Introduce object_class_property_deprecate()
On Mon, Jan 09, 2023 at 11:54:16PM +0100, Philippe Mathieu-Daudé wrote:
> Introduce object_class_property_deprecate() to register
> a QOM property as deprecated. When this property's getter /
> setter is called, a deprecation warning is displayed on the
> monitor.
>
> Inspired-by: Daniel P. Berrange
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> include/qom/object.h | 17 +
> qom/object.c | 23 +++
> 2 files changed, 40 insertions(+)
>
> diff --git a/include/qom/object.h b/include/qom/object.h
> index ef7258a5e1..b76724292c 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -97,6 +97,7 @@ struct ObjectProperty
> ObjectPropertyInit *init;
> void *opaque;
> QObject *defval;
> +const char *deprecation_reason;
> };
>
> /**
> @@ -1075,6 +1076,22 @@ ObjectProperty *object_class_property_add(ObjectClass
> *klass, const char *name,
>ObjectPropertyRelease *release,
>void *opaque);
>
> +/**
> + * object_class_property_deprecate:
> + * @klass: the class to add a property to
> + * @name: the name of the property. This can contain any character except
> for
> + * a forward slash. In general, you should use hyphens '-' instead of
> + * underscores '_' when naming properties.
> + * @reason: the deprecation reason.
> + * @version_major: the major version since this property is deprecated.
> + * @version_minor: the minor version since this property is deprecated.
> + *
> + * Deprecate a class property.
> + */
> +void object_class_property_deprecate(ObjectClass *klass,
> + const char *name, const char *reason,
> + int version_major, int version_minor);
> +
> /**
> * object_property_set_default_bool:
> * @prop: the property to set
> diff --git a/qom/object.c b/qom/object.c
> index e25f1e96db..05b97cd424 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1293,6 +1293,16 @@ object_class_property_add(ObjectClass *klass,
> return prop;
> }
>
> +void object_class_property_deprecate(ObjectClass *klass,
> + const char *name, const char *reason,
> + int version_major, int version_minor)
> +{
> +ObjectProperty *prop = object_class_property_find(klass, name);
> +
> +assert(prop);
> +prop->deprecation_reason = reason;
> +}
Nothing is using the 'version_major' / 'version_minor' parameters so
they look redundant.
> @@ -1392,6 +1413,7 @@ bool object_property_get(Object *obj, const char *name,
> Visitor *v,
> return false;
> }
>
> +object_property_check_deprecation(obj, name, prop);
> if (!prop->get) {
> error_setg(errp, "Property '%s.%s' is not readable",
> object_get_typename(obj), name);
> @@ -1412,6 +1434,7 @@ bool object_property_set(Object *obj, const char *name,
> Visitor *v,
> return false;
> }
>
> +object_property_check_deprecation(obj, name, prop);
> if (!prop->set) {
> error_setg(errp, "Property '%s.%s' is not writable",
> object_get_typename(obj), name);
> --
> 2.38.1
>
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PATCH v4 08/11] iotests: drop some extra ** in qmp() call
qmp() method supports passing dict (if it doesn't need substituting
'_' to '-' in keys). So, drop some extra '**' operators.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
tests/qemu-iotests/040| 4 +-
tests/qemu-iotests/041| 14 +++---
tests/qemu-iotests/129| 2 +-
tests/qemu-iotests/147| 2 +-
tests/qemu-iotests/155| 2 +-
tests/qemu-iotests/264| 12 ++---
tests/qemu-iotests/295| 5 +-
tests/qemu-iotests/296| 15 +++---
tests/qemu-iotests/tests/migrate-bitmaps-test | 4 +-
.../tests/mirror-ready-cancel-error | 50 +--
10 files changed, 54 insertions(+), 56 deletions(-)
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index 2b68946744..46a4ec9bde 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -774,7 +774,7 @@ class TestCommitWithFilters(iotests.QMPTestCase):
result = self.vm.qmp('object-add', qom_type='throttle-group', id='tg')
self.assert_qmp(result, 'return', {})
-result = self.vm.qmp('blockdev-add', **{
+result = self.vm.qmp('blockdev-add', {
'node-name': 'top-filter',
'driver': 'throttle',
'throttle-group': 'tg',
@@ -935,7 +935,7 @@ class TestCommitWithOverriddenBacking(iotests.QMPTestCase):
self.vm.launch()
# Use base_b instead of base_a as the backing of top
-result = self.vm.qmp('blockdev-add', **{
+result = self.vm.qmp('blockdev-add', {
'node-name': 'top',
'driver': iotests.imgfmt,
'file': {
diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index 550e4dc391..3aef42aec8 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -236,7 +236,7 @@ class TestSingleBlockdev(TestSingleDrive):
args = {'driver': iotests.imgfmt,
'node-name': self.qmp_target,
'file': { 'filename': target_img, 'driver': 'file' } }
-result = self.vm.qmp("blockdev-add", **args)
+result = self.vm.qmp("blockdev-add", args)
self.assert_qmp(result, 'return', {})
def test_mirror_to_self(self):
@@ -963,7 +963,7 @@ class TestRepairQuorum(iotests.QMPTestCase):
#assemble the quorum block device from the individual files
args = { "driver": "quorum", "node-name": "quorum0",
"vote-threshold": 2, "children": [ "img0", "img1", "img2" ] }
-result = self.vm.qmp("blockdev-add", **args)
+result = self.vm.qmp("blockdev-add", args)
self.assert_qmp(result, 'return', {})
@@ -1278,7 +1278,7 @@ class TestReplaces(iotests.QMPTestCase):
"""
Check that we can replace filter nodes.
"""
-result = self.vm.qmp('blockdev-add', **{
+result = self.vm.qmp('blockdev-add', {
'driver': 'copy-on-read',
'node-name': 'filter0',
'file': {
@@ -1319,7 +1319,7 @@ class TestFilters(iotests.QMPTestCase):
self.vm = iotests.VM().add_device('virtio-scsi,id=vio-scsi')
self.vm.launch()
-result = self.vm.qmp('blockdev-add', **{
+result = self.vm.qmp('blockdev-add', {
'node-name': 'target',
'driver': iotests.imgfmt,
'file': {
@@ -1355,7 +1355,7 @@ class TestFilters(iotests.QMPTestCase):
os.remove(backing_img)
def test_cor(self):
-result = self.vm.qmp('blockdev-add', **{
+result = self.vm.qmp('blockdev-add', {
'node-name': 'filter',
'driver': 'copy-on-read',
'file': self.filterless_chain
@@ -1384,7 +1384,7 @@ class TestFilters(iotests.QMPTestCase):
assert target_map[1]['depth'] == 0
def test_implicit_mirror_filter(self):
-result = self.vm.qmp('blockdev-add', **self.filterless_chain)
+result = self.vm.qmp('blockdev-add', self.filterless_chain)
self.assert_qmp(result, 'return', {})
# We need this so we can query from above the mirror node
@@ -1418,7 +1418,7 @@ class TestFilters(iotests.QMPTestCase):
def test_explicit_mirror_filter(self):
# Same test as above, but this time we give the mirror filter
# a node-name so it will not be invisible
-result = self.vm.qmp('blockdev-add', **self.filterless_chain)
+result = self.vm.qmp('blockdev-add', self.filterless_chain)
self.assert_qmp(result, 'return', {})
# We need this so we can query from above the mirror node
diff --git a/tests/qemu-iotests/129 b/tests/qemu-iotests/129
index
[PATCH v3 10/11] tests/vm/basevm.py: use cmd() instead of qmp()
We don't expect failure here and need 'result' object. cmd() is better
in this case.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
tests/vm/basevm.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 2276364c42..ff7e4fea15 100644
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -311,8 +311,8 @@ def boot(self, img, extra_args=[]):
self._guest = guest
# Init console so we can start consuming the chars.
self.console_init()
-usernet_info = guest.qmp("human-monitor-command",
- command_line="info usernet").get("return")
+usernet_info = guest.cmd("human-monitor-command",
+ command_line="info usernet")
self.ssh_port = get_info_usernet_hostfwd_port(usernet_info)
if not self.ssh_port:
raise Exception("Cannot find ssh port from 'info usernet':\n%s" % \
--
2.34.1
Re: [RFC PATCH 1/4] qom: Introduce object_class_property_deprecate()
On Mon, Jan 09, 2023 at 11:54:16PM +0100, Philippe Mathieu-Daudé wrote: > Introduce object_class_property_deprecate() to register > a QOM property as deprecated. When this property's getter / > setter is called, a deprecation warning is displayed on the > monitor. > > Inspired-by: Daniel P. Berrange > Signed-off-by: Philippe Mathieu-Daudé > --- > include/qom/object.h | 17 + > qom/object.c | 23 +++ > 2 files changed, 40 insertions(+) > > diff --git a/include/qom/object.h b/include/qom/object.h > index ef7258a5e1..b76724292c 100644 > --- a/include/qom/object.h > +++ b/include/qom/object.h > @@ -97,6 +97,7 @@ struct ObjectProperty > ObjectPropertyInit *init; > void *opaque; > QObject *defval; > +const char *deprecation_reason; > }; qapi/qom.list should change ObjectPropertyInfo type to include a 'deprecated': 'bool' field, as we've done for machine types and CPU models. Then qmp_qom_list should set this flag "deprecated = !!deprecation_reason" With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [RFC PATCH 4/4] qom: Warn when deprecated class property can be removed
Am 9. Januar 2023 22:54:19 UTC schrieb "Philippe Mathieu-Daudé"
:
>Per docs/system/deprecated.rst, a deprecated feature can be
>removed after 2 releases. Since we commit when a class property
>is deprecated, we can warn when the deprecation period is over.
>
>See also commit ef1f5b0a96 ("docs: clarify deprecation schedule").
>
>Signed-off-by: Philippe Mathieu-Daudé
>---
> qom/object.c | 7 +++
> 1 file changed, 7 insertions(+)
>
>diff --git a/qom/object.c b/qom/object.c
>index 05b97cd424..cb829f1e44 100644
>--- a/qom/object.c
>+++ b/qom/object.c
>@@ -17,6 +17,7 @@
> #include "qom/object_interfaces.h"
> #include "qemu/cutils.h"
> #include "qemu/memalign.h"
>+#include "qemu/qemu-version.h"
> #include "qapi/visitor.h"
> #include "qapi/string-input-visitor.h"
> #include "qapi/string-output-visitor.h"
>@@ -1300,6 +1301,12 @@ void object_class_property_deprecate(ObjectClass *klass,
> ObjectProperty *prop = object_class_property_find(klass, name);
>
> assert(prop);
>+if (qemu_version_delta_current(version_major, version_minor) <= 2) {
>+warn_report_once("Property '%s.%s' has been deprecated in release"
>+ " v%u.%u and can be removed.",
>+ object_class_get_name(klass), name,
>+ version_major, version_minor);
>+}
> prop->deprecation_reason = reason;
> }
>
Great series!
Perhaps turn object_class_property_deprecate() into a macro and warn at compile
time already? That way we won't miss removing any properties and users won't
see a warning they can't do anything about.
Best regards,
Bernhard
[PATCH v3 07/11] iotests: drop some occasional semicolons
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
tests/qemu-iotests/041 | 2 +-
tests/qemu-iotests/196 | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index 4d7a829b65..550e4dc391 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -1086,7 +1086,7 @@ class TestRepairQuorum(iotests.QMPTestCase):
def test_after_a_quorum_snapshot(self):
result = self.vm.qmp('blockdev-snapshot-sync', node_name='img1',
snapshot_file=quorum_snapshot_file,
- snapshot_node_name="snap1");
+ snapshot_node_name="snap1")
self.assert_qmp(result, 'return', {})
result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
diff --git a/tests/qemu-iotests/196 b/tests/qemu-iotests/196
index 76509a5ad1..27c1629be3 100755
--- a/tests/qemu-iotests/196
+++ b/tests/qemu-iotests/196
@@ -46,7 +46,7 @@ class TestInvalidateAutoclear(iotests.QMPTestCase):
def test_migration(self):
result = self.vm_a.qmp('migrate', uri='exec:cat>' + migfile)
-self.assert_qmp(result, 'return', {});
+self.assert_qmp(result, 'return', {})
self.assertNotEqual(self.vm_a.event_wait("STOP"), None)
with open(disk, 'r+b') as f:
--
2.34.1
Re: [RFC PATCH 4/4] qom: Warn when deprecated class property can be removed
On Mon, Jan 09, 2023 at 11:54:19PM +0100, Philippe Mathieu-Daudé wrote:
> Per docs/system/deprecated.rst, a deprecated feature can be
> removed after 2 releases. Since we commit when a class property
> is deprecated, we can warn when the deprecation period is over.
>
> See also commit ef1f5b0a96 ("docs: clarify deprecation schedule").
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> qom/object.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/qom/object.c b/qom/object.c
> index 05b97cd424..cb829f1e44 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -17,6 +17,7 @@
> #include "qom/object_interfaces.h"
> #include "qemu/cutils.h"
> #include "qemu/memalign.h"
> +#include "qemu/qemu-version.h"
> #include "qapi/visitor.h"
> #include "qapi/string-input-visitor.h"
> #include "qapi/string-output-visitor.h"
> @@ -1300,6 +1301,12 @@ void object_class_property_deprecate(ObjectClass
> *klass,
> ObjectProperty *prop = object_class_property_find(klass, name);
>
> assert(prop);
> +if (qemu_version_delta_current(version_major, version_minor) <= 2) {
> +warn_report_once("Property '%s.%s' has been deprecated in release"
> + " v%u.%u and can be removed.",
> + object_class_get_name(klass), name,
> + version_major, version_minor);
> +}
IMHO this is not really appropriate. Removal of deprecated features
is a job for maintainers, but this is printing a message at users.
Maintainers have a record of deprecations in the docs that they can
periodically review to identify stuff that can now be deleted.
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PATCH v4 03/11] python/machine.py: upgrade vm.cmd() method
The method is not popular in iotests, we prefer use vm.qmp() and then check success by hand.. But that's not optimal. To simplify movement to vm.cmd() let's support same interface improvements like in vm.qmp(). Signed-off-by: Vladimir Sementsov-Ogievskiy --- python/qemu/machine/machine.py | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py index 75f1f1c246..c45be440de 100644 --- a/python/qemu/machine/machine.py +++ b/python/qemu/machine/machine.py @@ -680,13 +680,23 @@ def qmp(self, cmd: str, return ret def cmd(self, cmd: str, -conv_keys: bool = True, +args_dict: Optional[Dict[str, object]] = None, +conv_keys: Optional[bool] = None, **args: Any) -> QMPReturnValue: """ Invoke a QMP command. On success return the response dict. On failure raise an exception. """ +if args_dict is not None: +assert not args +assert conv_keys is None +args = args_dict +conv_keys = False + +if conv_keys is None: +conv_keys = True + qmp_args = self._qmp_args(conv_keys, args) ret = self._qmp.cmd(cmd, **qmp_args) if cmd == 'quit': -- 2.34.1
[PATCH v4 06/11] iotests: refactor some common qmp result checks into generic pattern
To simplify further conversion.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
tests/qemu-iotests/040 | 3 ++-
tests/qemu-iotests/147 | 3 ++-
tests/qemu-iotests/155 | 4 ++--
tests/qemu-iotests/218 | 4 ++--
tests/qemu-iotests/296 | 3 ++-
5 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index 30eb97829e..2b68946744 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -62,9 +62,10 @@ class ImageCommitTestCase(iotests.QMPTestCase):
self.assert_no_active_block_jobs()
if node_names:
result = self.vm.qmp('block-commit', device='drive0',
top_node=top, base_node=base)
+self.assert_qmp(result, 'return', {})
else:
result = self.vm.qmp('block-commit', device='drive0', top=top,
base=base)
-self.assert_qmp(result, 'return', {})
+self.assert_qmp(result, 'return', {})
self.wait_for_complete(need_ready)
def run_default_commit_test(self):
diff --git a/tests/qemu-iotests/147 b/tests/qemu-iotests/147
index 47dfa62e6b..770b73e2f4 100755
--- a/tests/qemu-iotests/147
+++ b/tests/qemu-iotests/147
@@ -159,10 +159,11 @@ class BuiltinNBD(NBDBlockdevAddBase):
if export_name is None:
result = self.server.qmp('nbd-server-add', device='nbd-export')
+self.assert_qmp(result, 'return', {})
else:
result = self.server.qmp('nbd-server-add', device='nbd-export',
name=export_name)
-self.assert_qmp(result, 'return', {})
+self.assert_qmp(result, 'return', {})
if export_name2 is not None:
result = self.server.qmp('nbd-server-add', device='nbd-export',
diff --git a/tests/qemu-iotests/155 b/tests/qemu-iotests/155
index eadda52615..d3e1b7401e 100755
--- a/tests/qemu-iotests/155
+++ b/tests/qemu-iotests/155
@@ -181,6 +181,7 @@ class MirrorBaseClass(BaseClass):
result = self.vm.qmp(self.cmd, job_id='mirror-job',
device='source',
sync=sync, target='target',
auto_finalize=False)
+self.assert_qmp(result, 'return', {})
else:
if self.existing:
mode = 'existing'
@@ -190,8 +191,7 @@ class MirrorBaseClass(BaseClass):
sync=sync, target=target_img,
format=iotests.imgfmt, mode=mode,
node_name='target', auto_finalize=False)
-
-self.assert_qmp(result, 'return', {})
+self.assert_qmp(result, 'return', {})
self.vm.run_job('mirror-job', auto_finalize=False,
pre_finalize=self.openBacking, auto_dismiss=True)
diff --git a/tests/qemu-iotests/218 b/tests/qemu-iotests/218
index 6320c4cb56..5e74c55b6a 100755
--- a/tests/qemu-iotests/218
+++ b/tests/qemu-iotests/218
@@ -61,14 +61,14 @@ def start_mirror(vm, speed=None, buf_size=None):
sync='full',
speed=speed,
buf_size=buf_size)
+assert ret['return'] == {}
else:
ret = vm.qmp('blockdev-mirror',
job_id='mirror',
device='source',
target='target',
sync='full')
-
-assert ret['return'] == {}
+assert ret['return'] == {}
log('')
diff --git a/tests/qemu-iotests/296 b/tests/qemu-iotests/296
index 0d21b740a7..19a674c5ae 100755
--- a/tests/qemu-iotests/296
+++ b/tests/qemu-iotests/296
@@ -133,9 +133,10 @@ class EncryptionSetupTestCase(iotests.QMPTestCase):
if reOpen:
result = vm.qmp(command, options=[opts])
+self.assert_qmp(result, 'return', {})
else:
result = vm.qmp(command, **opts)
-self.assert_qmp(result, 'return', {})
+self.assert_qmp(result, 'return', {})
###
--
2.34.1
[PATCH v3 08/11] iotests: drop some extra ** in qmp() call
qmp() method supports passing dict (if it doesn't need substituting
'_' to '-' in keys). So, drop some extra '**' operators.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
tests/qemu-iotests/040| 4 +-
tests/qemu-iotests/041| 14 +++---
tests/qemu-iotests/129| 2 +-
tests/qemu-iotests/147| 2 +-
tests/qemu-iotests/155| 2 +-
tests/qemu-iotests/264| 12 ++---
tests/qemu-iotests/295| 5 +-
tests/qemu-iotests/296| 15 +++---
tests/qemu-iotests/tests/migrate-bitmaps-test | 4 +-
.../tests/mirror-ready-cancel-error | 50 +--
10 files changed, 54 insertions(+), 56 deletions(-)
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index 2b68946744..46a4ec9bde 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -774,7 +774,7 @@ class TestCommitWithFilters(iotests.QMPTestCase):
result = self.vm.qmp('object-add', qom_type='throttle-group', id='tg')
self.assert_qmp(result, 'return', {})
-result = self.vm.qmp('blockdev-add', **{
+result = self.vm.qmp('blockdev-add', {
'node-name': 'top-filter',
'driver': 'throttle',
'throttle-group': 'tg',
@@ -935,7 +935,7 @@ class TestCommitWithOverriddenBacking(iotests.QMPTestCase):
self.vm.launch()
# Use base_b instead of base_a as the backing of top
-result = self.vm.qmp('blockdev-add', **{
+result = self.vm.qmp('blockdev-add', {
'node-name': 'top',
'driver': iotests.imgfmt,
'file': {
diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index 550e4dc391..3aef42aec8 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -236,7 +236,7 @@ class TestSingleBlockdev(TestSingleDrive):
args = {'driver': iotests.imgfmt,
'node-name': self.qmp_target,
'file': { 'filename': target_img, 'driver': 'file' } }
-result = self.vm.qmp("blockdev-add", **args)
+result = self.vm.qmp("blockdev-add", args)
self.assert_qmp(result, 'return', {})
def test_mirror_to_self(self):
@@ -963,7 +963,7 @@ class TestRepairQuorum(iotests.QMPTestCase):
#assemble the quorum block device from the individual files
args = { "driver": "quorum", "node-name": "quorum0",
"vote-threshold": 2, "children": [ "img0", "img1", "img2" ] }
-result = self.vm.qmp("blockdev-add", **args)
+result = self.vm.qmp("blockdev-add", args)
self.assert_qmp(result, 'return', {})
@@ -1278,7 +1278,7 @@ class TestReplaces(iotests.QMPTestCase):
"""
Check that we can replace filter nodes.
"""
-result = self.vm.qmp('blockdev-add', **{
+result = self.vm.qmp('blockdev-add', {
'driver': 'copy-on-read',
'node-name': 'filter0',
'file': {
@@ -1319,7 +1319,7 @@ class TestFilters(iotests.QMPTestCase):
self.vm = iotests.VM().add_device('virtio-scsi,id=vio-scsi')
self.vm.launch()
-result = self.vm.qmp('blockdev-add', **{
+result = self.vm.qmp('blockdev-add', {
'node-name': 'target',
'driver': iotests.imgfmt,
'file': {
@@ -1355,7 +1355,7 @@ class TestFilters(iotests.QMPTestCase):
os.remove(backing_img)
def test_cor(self):
-result = self.vm.qmp('blockdev-add', **{
+result = self.vm.qmp('blockdev-add', {
'node-name': 'filter',
'driver': 'copy-on-read',
'file': self.filterless_chain
@@ -1384,7 +1384,7 @@ class TestFilters(iotests.QMPTestCase):
assert target_map[1]['depth'] == 0
def test_implicit_mirror_filter(self):
-result = self.vm.qmp('blockdev-add', **self.filterless_chain)
+result = self.vm.qmp('blockdev-add', self.filterless_chain)
self.assert_qmp(result, 'return', {})
# We need this so we can query from above the mirror node
@@ -1418,7 +1418,7 @@ class TestFilters(iotests.QMPTestCase):
def test_explicit_mirror_filter(self):
# Same test as above, but this time we give the mirror filter
# a node-name so it will not be invisible
-result = self.vm.qmp('blockdev-add', **self.filterless_chain)
+result = self.vm.qmp('blockdev-add', self.filterless_chain)
self.assert_qmp(result, 'return', {})
# We need this so we can query from above the mirror node
diff --git a/tests/qemu-iotests/129 b/tests/qemu-iotests/129
index
[PATCH v3 01/11] python: rename QEMUMonitorProtocol.cmd() to cmd_raw()
Having cmd() and command() methods in one class doesn't look good.
Rename cmd() to cmd_raw(), to show its meaning better.
We also want to rename command() to cmd() in future, so this commit is a
necessary first step.
Keep new cmd_raw() only in a few places where it's really needed and
move to command() where possible.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
python/qemu/machine/machine.py | 2 +-
python/qemu/qmp/legacy.py | 8 ++--
python/qemu/qmp/qmp_shell.py | 13 +++--
scripts/cpu-x86-uarch-abi.py | 12 ++--
tests/qemu-iotests/iotests.py | 2 +-
5 files changed, 17 insertions(+), 20 deletions(-)
diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index 748a0d807c..9059dc3948 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -674,7 +674,7 @@ def qmp(self, cmd: str,
conv_keys = True
qmp_args = self._qmp_args(conv_keys, args)
-ret = self._qmp.cmd(cmd, args=qmp_args)
+ret = self._qmp.cmd_raw(cmd, args=qmp_args)
if cmd == 'quit' and 'error' not in ret and 'return' in ret:
self._quit_issued = True
return ret
diff --git a/python/qemu/qmp/legacy.py b/python/qemu/qmp/legacy.py
index 1951754455..8e1a504052 100644
--- a/python/qemu/qmp/legacy.py
+++ b/python/qemu/qmp/legacy.py
@@ -186,21 +186,17 @@ def cmd_obj(self, qmp_cmd: QMPMessage) -> QMPMessage:
)
)
-def cmd(self, name: str,
-args: Optional[Dict[str, object]] = None,
-cmd_id: Optional[object] = None) -> QMPMessage:
+def cmd_raw(self, name: str,
+args: Optional[Dict[str, object]] = None) -> QMPMessage:
"""
Build a QMP command and send it to the QMP Monitor.
:param name: command name (string)
:param args: command arguments (dict)
-:param cmd_id: command id (dict, list, string or int)
"""
qmp_cmd: QMPMessage = {'execute': name}
if args:
qmp_cmd['arguments'] = args
-if cmd_id:
-qmp_cmd['id'] = cmd_id
return self.cmd_obj(qmp_cmd)
def command(self, cmd: str, **kwds: object) -> QMPReturnValue:
diff --git a/python/qemu/qmp/qmp_shell.py b/python/qemu/qmp/qmp_shell.py
index 619ab42ced..5c0d87a0ec 100644
--- a/python/qemu/qmp/qmp_shell.py
+++ b/python/qemu/qmp/qmp_shell.py
@@ -98,7 +98,7 @@
Sequence,
)
-from qemu.qmp import ConnectError, QMPError, SocketAddrT
+from qemu.qmp import ConnectError, QMPError, SocketAddrT, ExecuteError
from qemu.qmp.legacy import (
QEMUMonitorProtocol,
QMPBadPortError,
@@ -194,11 +194,12 @@ def close(self) -> None:
super().close()
def _fill_completion(self) -> None:
-cmds = self.cmd('query-commands')
-if 'error' in cmds:
-return
-for cmd in cmds['return']:
-self._completer.append(cmd['name'])
+try:
+cmds = self.command('query-commands')
+for cmd in cmds:
+self._completer.append(cmd['name'])
+except ExecuteError:
+pass
def _completer_setup(self) -> None:
self._completer = QMPCompleter()
diff --git a/scripts/cpu-x86-uarch-abi.py b/scripts/cpu-x86-uarch-abi.py
index 82ff07582f..893afd1b35 100644
--- a/scripts/cpu-x86-uarch-abi.py
+++ b/scripts/cpu-x86-uarch-abi.py
@@ -69,7 +69,7 @@
shell = QEMUMonitorProtocol(sock)
shell.connect()
-models = shell.cmd("query-cpu-definitions")
+models = shell.command("query-cpu-definitions")
# These QMP props don't correspond to CPUID fatures
# so ignore them
@@ -85,7 +85,7 @@
names = []
-for model in models["return"]:
+for model in models:
if "alias-of" in model:
continue
names.append(model["name"])
@@ -93,12 +93,12 @@
models = {}
for name in sorted(names):
-cpu = shell.cmd("query-cpu-model-expansion",
- { "type": "static",
- "model": { "name": name }})
+cpu = shell.command("query-cpu-model-expansion",
+{ "type": "static",
+ "model": { "name": name }})
got = {}
-for (feature, present) in cpu["return"]["model"]["props"].items():
+for (feature, present) in cpu["model"]["props"].items():
if present and feature not in skip:
got[feature] = True
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index da7d6637e1..c69b10ac82 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -460,7 +460,7 @@ def __init__(self, *args: str, instance_id: str = 'a', qmp:
bool = False):
def qmp(self, cmd: str, args: Optional[Dict[str, object]] = None) \
-> QMPMessage:
assert self._qmp is not None
-return self._qmp.cmd(cmd, args)
+return self._qmp.cmd_raw(cmd, args)
def stop(self, kill_signal=15):
self._p.send_signal(kill_signal)
--
Re: [PATCH v3 00/11] iotests: use vm.cmd()
Oops, no qemu-devel in CC. Sorry. Will resend -- Best regards, Vladimir
[PATCH v4 07/11] iotests: drop some occasional semicolons
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
tests/qemu-iotests/041 | 2 +-
tests/qemu-iotests/196 | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index 4d7a829b65..550e4dc391 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -1086,7 +1086,7 @@ class TestRepairQuorum(iotests.QMPTestCase):
def test_after_a_quorum_snapshot(self):
result = self.vm.qmp('blockdev-snapshot-sync', node_name='img1',
snapshot_file=quorum_snapshot_file,
- snapshot_node_name="snap1");
+ snapshot_node_name="snap1")
self.assert_qmp(result, 'return', {})
result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
diff --git a/tests/qemu-iotests/196 b/tests/qemu-iotests/196
index 76509a5ad1..27c1629be3 100755
--- a/tests/qemu-iotests/196
+++ b/tests/qemu-iotests/196
@@ -46,7 +46,7 @@ class TestInvalidateAutoclear(iotests.QMPTestCase):
def test_migration(self):
result = self.vm_a.qmp('migrate', uri='exec:cat>' + migfile)
-self.assert_qmp(result, 'return', {});
+self.assert_qmp(result, 'return', {})
self.assertNotEqual(self.vm_a.event_wait("STOP"), None)
with open(disk, 'r+b') as f:
--
2.34.1
[PATCH v4 01/11] python: rename QEMUMonitorProtocol.cmd() to cmd_raw()
Having cmd() and command() methods in one class doesn't look good.
Rename cmd() to cmd_raw(), to show its meaning better.
We also want to rename command() to cmd() in future, so this commit is a
necessary first step.
Keep new cmd_raw() only in a few places where it's really needed and
move to command() where possible.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
python/qemu/machine/machine.py | 2 +-
python/qemu/qmp/legacy.py | 8 ++--
python/qemu/qmp/qmp_shell.py | 13 +++--
scripts/cpu-x86-uarch-abi.py | 12 ++--
tests/qemu-iotests/iotests.py | 2 +-
5 files changed, 17 insertions(+), 20 deletions(-)
diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index 748a0d807c..9059dc3948 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -674,7 +674,7 @@ def qmp(self, cmd: str,
conv_keys = True
qmp_args = self._qmp_args(conv_keys, args)
-ret = self._qmp.cmd(cmd, args=qmp_args)
+ret = self._qmp.cmd_raw(cmd, args=qmp_args)
if cmd == 'quit' and 'error' not in ret and 'return' in ret:
self._quit_issued = True
return ret
diff --git a/python/qemu/qmp/legacy.py b/python/qemu/qmp/legacy.py
index 1951754455..8e1a504052 100644
--- a/python/qemu/qmp/legacy.py
+++ b/python/qemu/qmp/legacy.py
@@ -186,21 +186,17 @@ def cmd_obj(self, qmp_cmd: QMPMessage) -> QMPMessage:
)
)
-def cmd(self, name: str,
-args: Optional[Dict[str, object]] = None,
-cmd_id: Optional[object] = None) -> QMPMessage:
+def cmd_raw(self, name: str,
+args: Optional[Dict[str, object]] = None) -> QMPMessage:
"""
Build a QMP command and send it to the QMP Monitor.
:param name: command name (string)
:param args: command arguments (dict)
-:param cmd_id: command id (dict, list, string or int)
"""
qmp_cmd: QMPMessage = {'execute': name}
if args:
qmp_cmd['arguments'] = args
-if cmd_id:
-qmp_cmd['id'] = cmd_id
return self.cmd_obj(qmp_cmd)
def command(self, cmd: str, **kwds: object) -> QMPReturnValue:
diff --git a/python/qemu/qmp/qmp_shell.py b/python/qemu/qmp/qmp_shell.py
index 619ab42ced..5c0d87a0ec 100644
--- a/python/qemu/qmp/qmp_shell.py
+++ b/python/qemu/qmp/qmp_shell.py
@@ -98,7 +98,7 @@
Sequence,
)
-from qemu.qmp import ConnectError, QMPError, SocketAddrT
+from qemu.qmp import ConnectError, QMPError, SocketAddrT, ExecuteError
from qemu.qmp.legacy import (
QEMUMonitorProtocol,
QMPBadPortError,
@@ -194,11 +194,12 @@ def close(self) -> None:
super().close()
def _fill_completion(self) -> None:
-cmds = self.cmd('query-commands')
-if 'error' in cmds:
-return
-for cmd in cmds['return']:
-self._completer.append(cmd['name'])
+try:
+cmds = self.command('query-commands')
+for cmd in cmds:
+self._completer.append(cmd['name'])
+except ExecuteError:
+pass
def _completer_setup(self) -> None:
self._completer = QMPCompleter()
diff --git a/scripts/cpu-x86-uarch-abi.py b/scripts/cpu-x86-uarch-abi.py
index 82ff07582f..893afd1b35 100644
--- a/scripts/cpu-x86-uarch-abi.py
+++ b/scripts/cpu-x86-uarch-abi.py
@@ -69,7 +69,7 @@
shell = QEMUMonitorProtocol(sock)
shell.connect()
-models = shell.cmd("query-cpu-definitions")
+models = shell.command("query-cpu-definitions")
# These QMP props don't correspond to CPUID fatures
# so ignore them
@@ -85,7 +85,7 @@
names = []
-for model in models["return"]:
+for model in models:
if "alias-of" in model:
continue
names.append(model["name"])
@@ -93,12 +93,12 @@
models = {}
for name in sorted(names):
-cpu = shell.cmd("query-cpu-model-expansion",
- { "type": "static",
- "model": { "name": name }})
+cpu = shell.command("query-cpu-model-expansion",
+{ "type": "static",
+ "model": { "name": name }})
got = {}
-for (feature, present) in cpu["return"]["model"]["props"].items():
+for (feature, present) in cpu["model"]["props"].items():
if present and feature not in skip:
got[feature] = True
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index da7d6637e1..c69b10ac82 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -460,7 +460,7 @@ def __init__(self, *args: str, instance_id: str = 'a', qmp:
bool = False):
def qmp(self, cmd: str, args: Optional[Dict[str, object]] = None) \
-> QMPMessage:
assert self._qmp is not None
-return self._qmp.cmd(cmd, args)
+return self._qmp.cmd_raw(cmd, args)
def stop(self, kill_signal=15):
self._p.send_signal(kill_signal)
--
[PATCH v4 05/11] iotests: add some missed checks of qmp result
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
tests/qemu-iotests/041| 1 +
tests/qemu-iotests/151| 1 +
tests/qemu-iotests/152| 2 ++
tests/qemu-iotests/tests/migrate-bitmaps-test | 2 ++
4 files changed, 6 insertions(+)
diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index 8429958bf0..4d7a829b65 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -1087,6 +1087,7 @@ class TestRepairQuorum(iotests.QMPTestCase):
result = self.vm.qmp('blockdev-snapshot-sync', node_name='img1',
snapshot_file=quorum_snapshot_file,
snapshot_node_name="snap1");
+self.assert_qmp(result, 'return', {})
result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
sync='full', node_name='repair0', replaces="img1",
diff --git a/tests/qemu-iotests/151 b/tests/qemu-iotests/151
index b4d1bc2553..668d0c1e9c 100755
--- a/tests/qemu-iotests/151
+++ b/tests/qemu-iotests/151
@@ -159,6 +159,7 @@ class TestActiveMirror(iotests.QMPTestCase):
sync='full',
copy_mode='write-blocking',
speed=1)
+self.assert_qmp(result, 'return', {})
self.vm.hmp_qemu_io('source', 'break write_aio A')
self.vm.hmp_qemu_io('source', 'aio_write 0 1M') # 1
diff --git a/tests/qemu-iotests/152 b/tests/qemu-iotests/152
index 4e179c340f..b73a0d08a2 100755
--- a/tests/qemu-iotests/152
+++ b/tests/qemu-iotests/152
@@ -43,6 +43,7 @@ class TestUnaligned(iotests.QMPTestCase):
def test_unaligned(self):
result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
granularity=65536, target=target_img)
+self.assert_qmp(result, 'return', {})
self.complete_and_wait()
self.vm.shutdown()
self.assertEqual(iotests.image_size(test_img),
iotests.image_size(target_img),
@@ -51,6 +52,7 @@ class TestUnaligned(iotests.QMPTestCase):
def test_unaligned_with_update(self):
result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
granularity=65536, target=target_img)
+self.assert_qmp(result, 'return', {})
self.wait_ready()
self.vm.hmp_qemu_io('drive0', 'write 0 512')
self.complete_and_wait(wait_ready=False)
diff --git a/tests/qemu-iotests/tests/migrate-bitmaps-test
b/tests/qemu-iotests/tests/migrate-bitmaps-test
index 59f3357580..8668caae1e 100755
--- a/tests/qemu-iotests/tests/migrate-bitmaps-test
+++ b/tests/qemu-iotests/tests/migrate-bitmaps-test
@@ -101,6 +101,7 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
sha256 = get_bitmap_hash(self.vm_a)
result = self.vm_a.qmp('migrate', uri=mig_cmd)
+self.assert_qmp(result, 'return', {})
while True:
event = self.vm_a.event_wait('MIGRATION')
if event['data']['status'] == 'completed':
@@ -176,6 +177,7 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
self.assert_qmp(result, 'return', {})
result = self.vm_a.qmp('migrate', uri=mig_cmd)
+self.assert_qmp(result, 'return', {})
while True:
event = self.vm_a.event_wait('MIGRATION')
if event['data']['status'] == 'completed':
--
2.34.1
[PATCH v3 00/11] iotests: use vm.cmd()
Hi all!
Let's get rid of pattern
result = self.vm.qmp(...)
self.assert_qmp(result, 'return', {})
And switch to just
self.vm.cmd(...)
v3: rebase on master, fix some over-80 lines
Vladimir Sementsov-Ogievskiy (11):
python: rename QEMUMonitorProtocol.cmd() to cmd_raw()
python/qemu: rename command() to cmd()
python/machine.py: upgrade vm.cmd() method
iotests: QemuStorageDaemon: add cmd() method like in QEMUMachine.
iotests: add some missed checks of qmp result
iotests: refactor some common qmp result checks into generic pattern
iotests: drop some occasional semicolons
iotests: drop some extra ** in qmp() call
iotests.py: pause_job(): drop return value
tests/vm/basevm.py: use cmd() instead of qmp()
python: use vm.cmd() instead of vm.qmp() where appropriate
docs/devel/testing.rst| 10 +-
python/qemu/machine/machine.py| 20 +-
python/qemu/qmp/legacy.py | 10 +-
python/qemu/qmp/qmp_shell.py | 13 +-
python/qemu/utils/qemu_ga_client.py | 2 +-
python/qemu/utils/qom.py | 8 +-
python/qemu/utils/qom_common.py | 2 +-
python/qemu/utils/qom_fuse.py | 6 +-
scripts/cpu-x86-uarch-abi.py | 8 +-
scripts/device-crash-test | 8 +-
scripts/render_block_graph.py | 8 +-
tests/avocado/avocado_qemu/__init__.py| 4 +-
tests/avocado/cpu_queries.py | 5 +-
tests/avocado/hotplug_cpu.py | 10 +-
tests/avocado/info_usernet.py | 4 +-
tests/avocado/machine_arm_integratorcp.py | 6 +-
tests/avocado/machine_m68k_nextcube.py| 4 +-
tests/avocado/machine_mips_malta.py | 6 +-
tests/avocado/machine_s390_ccw_virtio.py | 28 +-
tests/avocado/migration.py| 10 +-
tests/avocado/pc_cpu_hotplug_props.py | 2 +-
tests/avocado/version.py | 4 +-
tests/avocado/virtio_check_params.py | 6 +-
tests/avocado/virtio_version.py | 5 +-
tests/avocado/vnc.py | 16 +-
tests/avocado/x86_cpu_model_versions.py | 13 +-
tests/migration/guestperf/engine.py | 150 +++---
tests/qemu-iotests/030| 168 +++---
tests/qemu-iotests/040| 171 +++
tests/qemu-iotests/041| 482 --
tests/qemu-iotests/045| 15 +-
tests/qemu-iotests/055| 62 +--
tests/qemu-iotests/056| 77 ++-
tests/qemu-iotests/093| 42 +-
tests/qemu-iotests/118| 225
tests/qemu-iotests/124| 102 ++--
tests/qemu-iotests/129| 14 +-
tests/qemu-iotests/132| 5 +-
tests/qemu-iotests/139| 45 +-
tests/qemu-iotests/147| 30 +-
tests/qemu-iotests/151| 56 +-
tests/qemu-iotests/152| 8 +-
tests/qemu-iotests/155| 55 +-
tests/qemu-iotests/165| 8 +-
tests/qemu-iotests/196| 3 +-
tests/qemu-iotests/205| 6 +-
tests/qemu-iotests/218| 105 ++--
tests/qemu-iotests/245| 245 -
tests/qemu-iotests/256| 34 +-
tests/qemu-iotests/257| 36 +-
tests/qemu-iotests/264| 31 +-
tests/qemu-iotests/281| 21 +-
tests/qemu-iotests/295| 16 +-
tests/qemu-iotests/296| 21 +-
tests/qemu-iotests/298| 13 +-
tests/qemu-iotests/300| 54 +-
tests/qemu-iotests/iotests.py | 18 +-
.../tests/export-incoming-iothread| 6 +-
.../qemu-iotests/tests/graph-changes-while-io | 6 +-
tests/qemu-iotests/tests/image-fleecing | 3 +-
.../tests/migrate-bitmaps-postcopy-test | 31 +-
tests/qemu-iotests/tests/migrate-bitmaps-test | 45 +-
.../qemu-iotests/tests/migrate-during-backup | 41 +-
.../qemu-iotests/tests/migration-permissions | 9 +-
.../tests/mirror-ready-cancel-error | 74 ++-
tests/qemu-iotests/tests/mirror-top-perms | 16 +-
tests/qemu-iotests/tests/nbd-multiconn| 12 +-
tests/qemu-iotests/tests/reopen-file | 3 +-
.../qemu-iotests/tests/stream-error-on-reset | 6 +-
tests/vm/basevm.py| 4 +-
70 files changed, 1179 insertions(+), 1613 deletions(-)
--
2.34.1
[PATCH v4 09/11] iotests.py: pause_job(): drop return value
The returned value is unused. It's simple to check by command
git grep -B 3 '\.pause_job('
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
tests/qemu-iotests/iotests.py | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index dd08cd8a2b..38f78dae3a 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -1321,8 +1321,7 @@ def pause_job(self, job_id='job0', wait=True):
result = self.vm.qmp('block-job-pause', device=job_id)
self.assert_qmp(result, 'return', {})
if wait:
-return self.pause_wait(job_id)
-return result
+self.pause_wait(job_id)
def case_skip(self, reason):
'''Skip this test case'''
--
2.34.1
[PATCH v4 10/11] tests/vm/basevm.py: use cmd() instead of qmp()
We don't expect failure here and need 'result' object. cmd() is better
in this case.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
tests/vm/basevm.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 2276364c42..ff7e4fea15 100644
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -311,8 +311,8 @@ def boot(self, img, extra_args=[]):
self._guest = guest
# Init console so we can start consuming the chars.
self.console_init()
-usernet_info = guest.qmp("human-monitor-command",
- command_line="info usernet").get("return")
+usernet_info = guest.cmd("human-monitor-command",
+ command_line="info usernet")
self.ssh_port = get_info_usernet_hostfwd_port(usernet_info)
if not self.ssh_port:
raise Exception("Cannot find ssh port from 'info usernet':\n%s" % \
--
2.34.1
[PATCH v4 00/11] iotests: use vm.cmd()
Hi all!
Let's get rid of pattern
result = self.vm.qmp(...)
self.assert_qmp(result, 'return', {})
And switch to just
self.vm.cmd(...)
v4: resend to fix CC
v3: rebase on master, fix some over-80 lines
Vladimir Sementsov-Ogievskiy (11):
python: rename QEMUMonitorProtocol.cmd() to cmd_raw()
python/qemu: rename command() to cmd()
python/machine.py: upgrade vm.cmd() method
iotests: QemuStorageDaemon: add cmd() method like in QEMUMachine.
iotests: add some missed checks of qmp result
iotests: refactor some common qmp result checks into generic pattern
iotests: drop some occasional semicolons
iotests: drop some extra ** in qmp() call
iotests.py: pause_job(): drop return value
tests/vm/basevm.py: use cmd() instead of qmp()
python: use vm.cmd() instead of vm.qmp() where appropriate
docs/devel/testing.rst| 10 +-
python/qemu/machine/machine.py| 20 +-
python/qemu/qmp/legacy.py | 10 +-
python/qemu/qmp/qmp_shell.py | 13 +-
python/qemu/utils/qemu_ga_client.py | 2 +-
python/qemu/utils/qom.py | 8 +-
python/qemu/utils/qom_common.py | 2 +-
python/qemu/utils/qom_fuse.py | 6 +-
scripts/cpu-x86-uarch-abi.py | 8 +-
scripts/device-crash-test | 8 +-
scripts/render_block_graph.py | 8 +-
tests/avocado/avocado_qemu/__init__.py| 4 +-
tests/avocado/cpu_queries.py | 5 +-
tests/avocado/hotplug_cpu.py | 10 +-
tests/avocado/info_usernet.py | 4 +-
tests/avocado/machine_arm_integratorcp.py | 6 +-
tests/avocado/machine_m68k_nextcube.py| 4 +-
tests/avocado/machine_mips_malta.py | 6 +-
tests/avocado/machine_s390_ccw_virtio.py | 28 +-
tests/avocado/migration.py| 10 +-
tests/avocado/pc_cpu_hotplug_props.py | 2 +-
tests/avocado/version.py | 4 +-
tests/avocado/virtio_check_params.py | 6 +-
tests/avocado/virtio_version.py | 5 +-
tests/avocado/vnc.py | 16 +-
tests/avocado/x86_cpu_model_versions.py | 13 +-
tests/migration/guestperf/engine.py | 150 +++---
tests/qemu-iotests/030| 168 +++---
tests/qemu-iotests/040| 171 +++
tests/qemu-iotests/041| 482 --
tests/qemu-iotests/045| 15 +-
tests/qemu-iotests/055| 62 +--
tests/qemu-iotests/056| 77 ++-
tests/qemu-iotests/093| 42 +-
tests/qemu-iotests/118| 225
tests/qemu-iotests/124| 102 ++--
tests/qemu-iotests/129| 14 +-
tests/qemu-iotests/132| 5 +-
tests/qemu-iotests/139| 45 +-
tests/qemu-iotests/147| 30 +-
tests/qemu-iotests/151| 56 +-
tests/qemu-iotests/152| 8 +-
tests/qemu-iotests/155| 55 +-
tests/qemu-iotests/165| 8 +-
tests/qemu-iotests/196| 3 +-
tests/qemu-iotests/205| 6 +-
tests/qemu-iotests/218| 105 ++--
tests/qemu-iotests/245| 245 -
tests/qemu-iotests/256| 34 +-
tests/qemu-iotests/257| 36 +-
tests/qemu-iotests/264| 31 +-
tests/qemu-iotests/281| 21 +-
tests/qemu-iotests/295| 16 +-
tests/qemu-iotests/296| 21 +-
tests/qemu-iotests/298| 13 +-
tests/qemu-iotests/300| 54 +-
tests/qemu-iotests/iotests.py | 18 +-
.../tests/export-incoming-iothread| 6 +-
.../qemu-iotests/tests/graph-changes-while-io | 6 +-
tests/qemu-iotests/tests/image-fleecing | 3 +-
.../tests/migrate-bitmaps-postcopy-test | 31 +-
tests/qemu-iotests/tests/migrate-bitmaps-test | 45 +-
.../qemu-iotests/tests/migrate-during-backup | 41 +-
.../qemu-iotests/tests/migration-permissions | 9 +-
.../tests/mirror-ready-cancel-error | 74 ++-
tests/qemu-iotests/tests/mirror-top-perms | 16 +-
tests/qemu-iotests/tests/nbd-multiconn| 12 +-
tests/qemu-iotests/tests/reopen-file | 3 +-
.../qemu-iotests/tests/stream-error-on-reset | 6 +-
tests/vm/basevm.py| 4 +-
70 files changed, 1179 insertions(+), 1613 deletions(-)
--
2.34.1
[PATCH v4 02/11] python/qemu: rename command() to cmd()
Use a shorter name. We are going to move in iotests from qmp() to
command() where possible. But command() is longer than qmp() and don't
look better. Let's rename.
You can simply grep for '\.command(' and for 'def command(' to check
that everything is updated (command() in tests/docker/docker.py is
unrelated).
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
docs/devel/testing.rst| 10 +-
python/qemu/machine/machine.py| 8 +-
python/qemu/qmp/legacy.py | 2 +-
python/qemu/qmp/qmp_shell.py | 2 +-
python/qemu/utils/qemu_ga_client.py | 2 +-
python/qemu/utils/qom.py | 8 +-
python/qemu/utils/qom_common.py | 2 +-
python/qemu/utils/qom_fuse.py | 6 +-
scripts/cpu-x86-uarch-abi.py | 8 +-
scripts/device-crash-test | 8 +-
scripts/render_block_graph.py | 8 +-
tests/avocado/avocado_qemu/__init__.py| 4 +-
tests/avocado/cpu_queries.py | 5 +-
tests/avocado/hotplug_cpu.py | 10 +-
tests/avocado/info_usernet.py | 4 +-
tests/avocado/machine_arm_integratorcp.py | 6 +-
tests/avocado/machine_m68k_nextcube.py| 4 +-
tests/avocado/machine_mips_malta.py | 6 +-
tests/avocado/machine_s390_ccw_virtio.py | 28 ++--
tests/avocado/migration.py| 10 +-
tests/avocado/pc_cpu_hotplug_props.py | 2 +-
tests/avocado/version.py | 4 +-
tests/avocado/virtio_check_params.py | 6 +-
tests/avocado/virtio_version.py | 5 +-
tests/avocado/x86_cpu_model_versions.py | 13 +-
tests/migration/guestperf/engine.py | 150 +++---
tests/qemu-iotests/256| 34 ++---
tests/qemu-iotests/257| 36 +++---
28 files changed, 198 insertions(+), 193 deletions(-)
diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index e10c47b5a7..e3d7bb218c 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -988,8 +988,8 @@ class. Here's a simple usage example:
"""
def test_qmp_human_info_version(self):
self.vm.launch()
- res = self.vm.command('human-monitor-command',
-command_line='info version')
+ res = self.vm.cmd('human-monitor-command',
+command_line='info version')
self.assertRegexpMatches(res, r'^(\d+\.\d+\.\d)')
To execute your test, run:
@@ -1039,15 +1039,15 @@ and hypothetical example follows:
first_machine.launch()
second_machine.launch()
- first_res = first_machine.command(
+ first_res = first_machine.cmd(
'human-monitor-command',
command_line='info version')
- second_res = second_machine.command(
+ second_res = second_machine.cmd(
'human-monitor-command',
command_line='info version')
- third_res = self.get_vm(name='third_machine').command(
+ third_res = self.get_vm(name='third_machine').cmd(
'human-monitor-command',
command_line='info version')
diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index 9059dc3948..75f1f1c246 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -679,16 +679,16 @@ def qmp(self, cmd: str,
self._quit_issued = True
return ret
-def command(self, cmd: str,
-conv_keys: bool = True,
-**args: Any) -> QMPReturnValue:
+def cmd(self, cmd: str,
+conv_keys: bool = True,
+**args: Any) -> QMPReturnValue:
"""
Invoke a QMP command.
On success return the response dict.
On failure raise an exception.
"""
qmp_args = self._qmp_args(conv_keys, args)
-ret = self._qmp.command(cmd, **qmp_args)
+ret = self._qmp.cmd(cmd, **qmp_args)
if cmd == 'quit':
self._quit_issued = True
return ret
diff --git a/python/qemu/qmp/legacy.py b/python/qemu/qmp/legacy.py
index 8e1a504052..f5b4b1a717 100644
--- a/python/qemu/qmp/legacy.py
+++ b/python/qemu/qmp/legacy.py
@@ -199,7 +199,7 @@ def cmd_raw(self, name: str,
qmp_cmd['arguments'] = args
return self.cmd_obj(qmp_cmd)
-def command(self, cmd: str, **kwds: object) -> QMPReturnValue:
+def cmd(self, cmd: str, **kwds: object) -> QMPReturnValue:
"""
Build and send a QMP command to the monitor, report errors if any
"""
diff --git a/python/qemu/qmp/qmp_shell.py b/python/qemu/qmp/qmp_shell.py
index 5c0d87a0ec..5c578b50e2 100644
--- a/python/qemu/qmp/qmp_shell.py
+++ b/python/qemu/qmp/qmp_shell.py
@@ -195,7 +195,7 @@ def close(self) -> None:
def _fill_completion(self) -> None:
try:
-cmds = self.command('query-commands')
+
[PATCH v4 04/11] iotests: QemuStorageDaemon: add cmd() method like in QEMUMachine.
Add similar method for consistency. Signed-off-by: Vladimir Sementsov-Ogievskiy --- tests/qemu-iotests/iotests.py | 4 1 file changed, 4 insertions(+) diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index c69b10ac82..dd08cd8a2b 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -462,6 +462,10 @@ def qmp(self, cmd: str, args: Optional[Dict[str, object]] = None) \ assert self._qmp is not None return self._qmp.cmd_raw(cmd, args) +def cmd(self, cmd: str, args: Optional[Dict[str, object]] = None) \ +-> QMPMessage: +return self._qmp.cmd(cmd, **args) + def stop(self, kill_signal=15): self._p.send_signal(kill_signal) self._p.wait() -- 2.34.1
[PATCH v3 09/11] iotests.py: pause_job(): drop return value
The returned value is unused. It's simple to check by command
git grep -B 3 '\.pause_job('
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
tests/qemu-iotests/iotests.py | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index dd08cd8a2b..38f78dae3a 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -1321,8 +1321,7 @@ def pause_job(self, job_id='job0', wait=True):
result = self.vm.qmp('block-job-pause', device=job_id)
self.assert_qmp(result, 'return', {})
if wait:
-return self.pause_wait(job_id)
-return result
+self.pause_wait(job_id)
def case_skip(self, reason):
'''Skip this test case'''
--
2.34.1
[PATCH v3 05/11] iotests: add some missed checks of qmp result
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
tests/qemu-iotests/041| 1 +
tests/qemu-iotests/151| 1 +
tests/qemu-iotests/152| 2 ++
tests/qemu-iotests/tests/migrate-bitmaps-test | 2 ++
4 files changed, 6 insertions(+)
diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index 8429958bf0..4d7a829b65 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -1087,6 +1087,7 @@ class TestRepairQuorum(iotests.QMPTestCase):
result = self.vm.qmp('blockdev-snapshot-sync', node_name='img1',
snapshot_file=quorum_snapshot_file,
snapshot_node_name="snap1");
+self.assert_qmp(result, 'return', {})
result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
sync='full', node_name='repair0', replaces="img1",
diff --git a/tests/qemu-iotests/151 b/tests/qemu-iotests/151
index b4d1bc2553..668d0c1e9c 100755
--- a/tests/qemu-iotests/151
+++ b/tests/qemu-iotests/151
@@ -159,6 +159,7 @@ class TestActiveMirror(iotests.QMPTestCase):
sync='full',
copy_mode='write-blocking',
speed=1)
+self.assert_qmp(result, 'return', {})
self.vm.hmp_qemu_io('source', 'break write_aio A')
self.vm.hmp_qemu_io('source', 'aio_write 0 1M') # 1
diff --git a/tests/qemu-iotests/152 b/tests/qemu-iotests/152
index 4e179c340f..b73a0d08a2 100755
--- a/tests/qemu-iotests/152
+++ b/tests/qemu-iotests/152
@@ -43,6 +43,7 @@ class TestUnaligned(iotests.QMPTestCase):
def test_unaligned(self):
result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
granularity=65536, target=target_img)
+self.assert_qmp(result, 'return', {})
self.complete_and_wait()
self.vm.shutdown()
self.assertEqual(iotests.image_size(test_img),
iotests.image_size(target_img),
@@ -51,6 +52,7 @@ class TestUnaligned(iotests.QMPTestCase):
def test_unaligned_with_update(self):
result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
granularity=65536, target=target_img)
+self.assert_qmp(result, 'return', {})
self.wait_ready()
self.vm.hmp_qemu_io('drive0', 'write 0 512')
self.complete_and_wait(wait_ready=False)
diff --git a/tests/qemu-iotests/tests/migrate-bitmaps-test
b/tests/qemu-iotests/tests/migrate-bitmaps-test
index 59f3357580..8668caae1e 100755
--- a/tests/qemu-iotests/tests/migrate-bitmaps-test
+++ b/tests/qemu-iotests/tests/migrate-bitmaps-test
@@ -101,6 +101,7 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
sha256 = get_bitmap_hash(self.vm_a)
result = self.vm_a.qmp('migrate', uri=mig_cmd)
+self.assert_qmp(result, 'return', {})
while True:
event = self.vm_a.event_wait('MIGRATION')
if event['data']['status'] == 'completed':
@@ -176,6 +177,7 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
self.assert_qmp(result, 'return', {})
result = self.vm_a.qmp('migrate', uri=mig_cmd)
+self.assert_qmp(result, 'return', {})
while True:
event = self.vm_a.event_wait('MIGRATION')
if event['data']['status'] == 'completed':
--
2.34.1
[PATCH v3 06/11] iotests: refactor some common qmp result checks into generic pattern
To simplify further conversion.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
tests/qemu-iotests/040 | 3 ++-
tests/qemu-iotests/147 | 3 ++-
tests/qemu-iotests/155 | 4 ++--
tests/qemu-iotests/218 | 4 ++--
tests/qemu-iotests/296 | 3 ++-
5 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index 30eb97829e..2b68946744 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -62,9 +62,10 @@ class ImageCommitTestCase(iotests.QMPTestCase):
self.assert_no_active_block_jobs()
if node_names:
result = self.vm.qmp('block-commit', device='drive0',
top_node=top, base_node=base)
+self.assert_qmp(result, 'return', {})
else:
result = self.vm.qmp('block-commit', device='drive0', top=top,
base=base)
-self.assert_qmp(result, 'return', {})
+self.assert_qmp(result, 'return', {})
self.wait_for_complete(need_ready)
def run_default_commit_test(self):
diff --git a/tests/qemu-iotests/147 b/tests/qemu-iotests/147
index 47dfa62e6b..770b73e2f4 100755
--- a/tests/qemu-iotests/147
+++ b/tests/qemu-iotests/147
@@ -159,10 +159,11 @@ class BuiltinNBD(NBDBlockdevAddBase):
if export_name is None:
result = self.server.qmp('nbd-server-add', device='nbd-export')
+self.assert_qmp(result, 'return', {})
else:
result = self.server.qmp('nbd-server-add', device='nbd-export',
name=export_name)
-self.assert_qmp(result, 'return', {})
+self.assert_qmp(result, 'return', {})
if export_name2 is not None:
result = self.server.qmp('nbd-server-add', device='nbd-export',
diff --git a/tests/qemu-iotests/155 b/tests/qemu-iotests/155
index eadda52615..d3e1b7401e 100755
--- a/tests/qemu-iotests/155
+++ b/tests/qemu-iotests/155
@@ -181,6 +181,7 @@ class MirrorBaseClass(BaseClass):
result = self.vm.qmp(self.cmd, job_id='mirror-job',
device='source',
sync=sync, target='target',
auto_finalize=False)
+self.assert_qmp(result, 'return', {})
else:
if self.existing:
mode = 'existing'
@@ -190,8 +191,7 @@ class MirrorBaseClass(BaseClass):
sync=sync, target=target_img,
format=iotests.imgfmt, mode=mode,
node_name='target', auto_finalize=False)
-
-self.assert_qmp(result, 'return', {})
+self.assert_qmp(result, 'return', {})
self.vm.run_job('mirror-job', auto_finalize=False,
pre_finalize=self.openBacking, auto_dismiss=True)
diff --git a/tests/qemu-iotests/218 b/tests/qemu-iotests/218
index 6320c4cb56..5e74c55b6a 100755
--- a/tests/qemu-iotests/218
+++ b/tests/qemu-iotests/218
@@ -61,14 +61,14 @@ def start_mirror(vm, speed=None, buf_size=None):
sync='full',
speed=speed,
buf_size=buf_size)
+assert ret['return'] == {}
else:
ret = vm.qmp('blockdev-mirror',
job_id='mirror',
device='source',
target='target',
sync='full')
-
-assert ret['return'] == {}
+assert ret['return'] == {}
log('')
diff --git a/tests/qemu-iotests/296 b/tests/qemu-iotests/296
index 0d21b740a7..19a674c5ae 100755
--- a/tests/qemu-iotests/296
+++ b/tests/qemu-iotests/296
@@ -133,9 +133,10 @@ class EncryptionSetupTestCase(iotests.QMPTestCase):
if reOpen:
result = vm.qmp(command, options=[opts])
+self.assert_qmp(result, 'return', {})
else:
result = vm.qmp(command, **opts)
-self.assert_qmp(result, 'return', {})
+self.assert_qmp(result, 'return', {})
###
--
2.34.1
[PATCH v3 02/11] python/qemu: rename command() to cmd()
Use a shorter name. We are going to move in iotests from qmp() to
command() where possible. But command() is longer than qmp() and don't
look better. Let's rename.
You can simply grep for '\.command(' and for 'def command(' to check
that everything is updated (command() in tests/docker/docker.py is
unrelated).
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
docs/devel/testing.rst| 10 +-
python/qemu/machine/machine.py| 8 +-
python/qemu/qmp/legacy.py | 2 +-
python/qemu/qmp/qmp_shell.py | 2 +-
python/qemu/utils/qemu_ga_client.py | 2 +-
python/qemu/utils/qom.py | 8 +-
python/qemu/utils/qom_common.py | 2 +-
python/qemu/utils/qom_fuse.py | 6 +-
scripts/cpu-x86-uarch-abi.py | 8 +-
scripts/device-crash-test | 8 +-
scripts/render_block_graph.py | 8 +-
tests/avocado/avocado_qemu/__init__.py| 4 +-
tests/avocado/cpu_queries.py | 5 +-
tests/avocado/hotplug_cpu.py | 10 +-
tests/avocado/info_usernet.py | 4 +-
tests/avocado/machine_arm_integratorcp.py | 6 +-
tests/avocado/machine_m68k_nextcube.py| 4 +-
tests/avocado/machine_mips_malta.py | 6 +-
tests/avocado/machine_s390_ccw_virtio.py | 28 ++--
tests/avocado/migration.py| 10 +-
tests/avocado/pc_cpu_hotplug_props.py | 2 +-
tests/avocado/version.py | 4 +-
tests/avocado/virtio_check_params.py | 6 +-
tests/avocado/virtio_version.py | 5 +-
tests/avocado/x86_cpu_model_versions.py | 13 +-
tests/migration/guestperf/engine.py | 150 +++---
tests/qemu-iotests/256| 34 ++---
tests/qemu-iotests/257| 36 +++---
28 files changed, 198 insertions(+), 193 deletions(-)
diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index e10c47b5a7..e3d7bb218c 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -988,8 +988,8 @@ class. Here's a simple usage example:
"""
def test_qmp_human_info_version(self):
self.vm.launch()
- res = self.vm.command('human-monitor-command',
-command_line='info version')
+ res = self.vm.cmd('human-monitor-command',
+command_line='info version')
self.assertRegexpMatches(res, r'^(\d+\.\d+\.\d)')
To execute your test, run:
@@ -1039,15 +1039,15 @@ and hypothetical example follows:
first_machine.launch()
second_machine.launch()
- first_res = first_machine.command(
+ first_res = first_machine.cmd(
'human-monitor-command',
command_line='info version')
- second_res = second_machine.command(
+ second_res = second_machine.cmd(
'human-monitor-command',
command_line='info version')
- third_res = self.get_vm(name='third_machine').command(
+ third_res = self.get_vm(name='third_machine').cmd(
'human-monitor-command',
command_line='info version')
diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index 9059dc3948..75f1f1c246 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -679,16 +679,16 @@ def qmp(self, cmd: str,
self._quit_issued = True
return ret
-def command(self, cmd: str,
-conv_keys: bool = True,
-**args: Any) -> QMPReturnValue:
+def cmd(self, cmd: str,
+conv_keys: bool = True,
+**args: Any) -> QMPReturnValue:
"""
Invoke a QMP command.
On success return the response dict.
On failure raise an exception.
"""
qmp_args = self._qmp_args(conv_keys, args)
-ret = self._qmp.command(cmd, **qmp_args)
+ret = self._qmp.cmd(cmd, **qmp_args)
if cmd == 'quit':
self._quit_issued = True
return ret
diff --git a/python/qemu/qmp/legacy.py b/python/qemu/qmp/legacy.py
index 8e1a504052..f5b4b1a717 100644
--- a/python/qemu/qmp/legacy.py
+++ b/python/qemu/qmp/legacy.py
@@ -199,7 +199,7 @@ def cmd_raw(self, name: str,
qmp_cmd['arguments'] = args
return self.cmd_obj(qmp_cmd)
-def command(self, cmd: str, **kwds: object) -> QMPReturnValue:
+def cmd(self, cmd: str, **kwds: object) -> QMPReturnValue:
"""
Build and send a QMP command to the monitor, report errors if any
"""
diff --git a/python/qemu/qmp/qmp_shell.py b/python/qemu/qmp/qmp_shell.py
index 5c0d87a0ec..5c578b50e2 100644
--- a/python/qemu/qmp/qmp_shell.py
+++ b/python/qemu/qmp/qmp_shell.py
@@ -195,7 +195,7 @@ def close(self) -> None:
def _fill_completion(self) -> None:
try:
-cmds = self.command('query-commands')
+
[PATCH v3 04/11] iotests: QemuStorageDaemon: add cmd() method like in QEMUMachine.
Add similar method for consistency. Signed-off-by: Vladimir Sementsov-Ogievskiy --- tests/qemu-iotests/iotests.py | 4 1 file changed, 4 insertions(+) diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index c69b10ac82..dd08cd8a2b 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -462,6 +462,10 @@ def qmp(self, cmd: str, args: Optional[Dict[str, object]] = None) \ assert self._qmp is not None return self._qmp.cmd_raw(cmd, args) +def cmd(self, cmd: str, args: Optional[Dict[str, object]] = None) \ +-> QMPMessage: +return self._qmp.cmd(cmd, **args) + def stop(self, kill_signal=15): self._p.send_signal(kill_signal) self._p.wait() -- 2.34.1
[PATCH v3 03/11] python/machine.py: upgrade vm.cmd() method
The method is not popular in iotests, we prefer use vm.qmp() and then check success by hand.. But that's not optimal. To simplify movement to vm.cmd() let's support same interface improvements like in vm.qmp(). Signed-off-by: Vladimir Sementsov-Ogievskiy --- python/qemu/machine/machine.py | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py index 75f1f1c246..c45be440de 100644 --- a/python/qemu/machine/machine.py +++ b/python/qemu/machine/machine.py @@ -680,13 +680,23 @@ def qmp(self, cmd: str, return ret def cmd(self, cmd: str, -conv_keys: bool = True, +args_dict: Optional[Dict[str, object]] = None, +conv_keys: Optional[bool] = None, **args: Any) -> QMPReturnValue: """ Invoke a QMP command. On success return the response dict. On failure raise an exception. """ +if args_dict is not None: +assert not args +assert conv_keys is None +args = args_dict +conv_keys = False + +if conv_keys is None: +conv_keys = True + qmp_args = self._qmp_args(conv_keys, args) ret = self._qmp.cmd(cmd, **qmp_args) if cmd == 'quit': -- 2.34.1
