From f62492bb8d1ea7f7e156ffbdf411de46107072c5 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 13 Aug 2018 15:23:49 +0200 Subject: [PATCH 01/21] block/qapi: Fix memory leak in qmp_query_blockstats() For BlockBackends that are skipped in query-blockstats, we would leak info since commit 567dcb31. Allocate info only later to avoid the memory leak. Fixes: CID 1394727 Cc: qemu-stable@nongnu.org Signed-off-by: Kevin Wolf Reviewed-by: Alberto Garcia --- block/qapi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/block/qapi.c b/block/qapi.c index 339727f0f4..c66f949db8 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -594,7 +594,7 @@ BlockStatsList *qmp_query_blockstats(bool has_query_nodes, } } else { for (blk = blk_all_next(NULL); blk; blk = blk_all_next(blk)) { - BlockStatsList *info = g_malloc0(sizeof(*info)); + BlockStatsList *info; AioContext *ctx = blk_get_aio_context(blk); BlockStats *s; char *qdev; @@ -619,6 +619,7 @@ BlockStatsList *qmp_query_blockstats(bool has_query_nodes, bdrv_query_blk_stats(s->stats, blk); aio_context_release(ctx); + info = g_malloc0(sizeof(*info)); info->value = s; *p_next = info; p_next = &info->next; From ef7a6a3c2a7725b169d054aa7487f9738bd6c4a6 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Thu, 2 Aug 2018 17:50:23 +0300 Subject: [PATCH 02/21] qemu-iotests: Test removing a throttle group member with a pending timer A throttle group can have several members, and each one of them can have several pending requests in the queue. The requests are processed in a round-robin fashion, so the algorithm decides the drive that is going to run the next request and sets a timer in it. Once the timer fires and the throttled request is run then the next drive from the group is selected and a new timer is set. If the user tried to remove a drive from a group and that drive had a timer set then the code was not taking care of setting up a new timer in one of the remaining members of the group, freezing their I/O. This problem was fixed in 6fccbb475bc6effc313ee9481726a1748b6dae57, and this patch adds a new test case that reproduces this exact scenario. Signed-off-by: Alberto Garcia Signed-off-by: Kevin Wolf --- tests/qemu-iotests/093 | 52 ++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/093.out | 4 +-- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093 index 68e344f8c1..b26cd34e32 100755 --- a/tests/qemu-iotests/093 +++ b/tests/qemu-iotests/093 @@ -208,6 +208,58 @@ class ThrottleTestCase(iotests.QMPTestCase): limits[tk] = rate self.do_test_throttle(ndrives, 5, limits) + # Test that removing a drive from a throttle group should not + # affect the remaining members of the group. + # https://bugzilla.redhat.com/show_bug.cgi?id=1535914 + def test_remove_group_member(self): + # Create a throttle group with two drives + # and set a 4 KB/s read limit. + params = {"bps": 0, + "bps_rd": 4096, + "bps_wr": 0, + "iops": 0, + "iops_rd": 0, + "iops_wr": 0 } + self.configure_throttle(2, params) + + # Read 4KB from drive0. This is performed immediately. + self.vm.hmp_qemu_io("drive0", "aio_read 0 4096") + + # Read 4KB again. The I/O limit has been exceeded so this + # request is throttled and a timer is set to wake it up. + self.vm.hmp_qemu_io("drive0", "aio_read 0 4096") + + # Read from drive1. We're still over the I/O limit so this + # request is also throttled. There's no timer set in drive1 + # because there's already one in drive0. Once the timer in + # drive0 fires and its throttled request is processed then the + # next request in the queue will be scheduled: this one. + self.vm.hmp_qemu_io("drive1", "aio_read 0 4096") + + # At this point only the first 4KB have been read from drive0. + # The other requests are throttled. + self.assertEqual(self.blockstats('drive0')[0], 4096) + self.assertEqual(self.blockstats('drive1')[0], 0) + + # Remove drive0 from the throttle group and disable its I/O limits. + # drive1 remains in the group with a throttled request. + params['bps_rd'] = 0 + params['device'] = 'drive0' + result = self.vm.qmp("block_set_io_throttle", conv_keys=False, **params) + self.assert_qmp(result, 'return', {}) + + # Removing the I/O limits from drive0 drains its pending request. + # The read request in drive1 is still throttled. + self.assertEqual(self.blockstats('drive0')[0], 8192) + self.assertEqual(self.blockstats('drive1')[0], 0) + + # Advance the clock 5 seconds. This completes the request in drive1 + self.vm.qtest("clock_step %d" % (5 * nsec_per_sec)) + + # Now all requests have been processed. + self.assertEqual(self.blockstats('drive0')[0], 8192) + self.assertEqual(self.blockstats('drive1')[0], 4096) + class ThrottleTestCoroutine(ThrottleTestCase): test_img = "null-co://" diff --git a/tests/qemu-iotests/093.out b/tests/qemu-iotests/093.out index 594c16f49f..36376bed87 100644 --- a/tests/qemu-iotests/093.out +++ b/tests/qemu-iotests/093.out @@ -1,5 +1,5 @@ -........ +.......... ---------------------------------------------------------------------- -Ran 8 tests +Ran 10 tests OK From 5d8e4ca035f5a21e8634eb63a678bed55a1a94f9 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Thu, 2 Aug 2018 17:50:24 +0300 Subject: [PATCH 03/21] throttle-groups: Skip the round-robin if a member is being drained In the throttling code after an I/O request has been completed the next one is selected from a different member using a round-robin algorithm. This ensures that all members get a chance to finish their pending I/O requests. However, if a group member has its I/O limits disabled (because it's being drained) then we should always give it priority in order to have all its pending requests finished as soon as possible. If we don't do this we could have a member in the process of being drained waiting for the throttled requests of other members, for which the I/O limits still apply. This can have additional consequences: if we're running in qtest mode (with QEMU_CLOCK_VIRTUAL) then timers can only fire if we advance the clock manually, so attempting to drain a block device can hang QEMU in the BDRV_POLL_WHILE() loop at the end of bdrv_do_drained_begin(). Signed-off-by: Alberto Garcia Signed-off-by: Kevin Wolf --- block/throttle-groups.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/block/throttle-groups.c b/block/throttle-groups.c index e297b04e17..d46c56b31e 100644 --- a/block/throttle-groups.c +++ b/block/throttle-groups.c @@ -221,6 +221,15 @@ static ThrottleGroupMember *next_throttle_token(ThrottleGroupMember *tgm, ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts); ThrottleGroupMember *token, *start; + /* If this member has its I/O limits disabled then it means that + * it's being drained. Skip the round-robin search and return tgm + * immediately if it has pending requests. Otherwise we could be + * forcing it to wait for other member's throttled requests. */ + if (tgm_has_pending_reqs(tgm, is_write) && + atomic_read(&tgm->io_limits_disabled)) { + return tgm; + } + start = token = tg->tokens[is_write]; /* get next bs round in round robin style */ From 3db3e9c621519cbfc2b52e98b38f13ad863c0062 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Thu, 2 Aug 2018 17:50:25 +0300 Subject: [PATCH 04/21] qemu-iotests: Update 093 to improve the draining test The previous patch fixes a problem in which draining a block device with more than one throttled request can make it wait first for the completion of requests in other members of the same group. This patch updates test_remove_group_member() in iotest 093 to reproduce that scenario. This updated test would hang QEMU without the fix from the previous patch. Signed-off-by: Alberto Garcia Signed-off-by: Kevin Wolf --- tests/qemu-iotests/093 | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093 index b26cd34e32..9d1971a56c 100755 --- a/tests/qemu-iotests/093 +++ b/tests/qemu-iotests/093 @@ -225,15 +225,18 @@ class ThrottleTestCase(iotests.QMPTestCase): # Read 4KB from drive0. This is performed immediately. self.vm.hmp_qemu_io("drive0", "aio_read 0 4096") - # Read 4KB again. The I/O limit has been exceeded so this + # Read 2KB. The I/O limit has been exceeded so this # request is throttled and a timer is set to wake it up. - self.vm.hmp_qemu_io("drive0", "aio_read 0 4096") + self.vm.hmp_qemu_io("drive0", "aio_read 0 2048") - # Read from drive1. We're still over the I/O limit so this - # request is also throttled. There's no timer set in drive1 - # because there's already one in drive0. Once the timer in - # drive0 fires and its throttled request is processed then the - # next request in the queue will be scheduled: this one. + # Read 2KB again. We're still over the I/O limit so this is + # request is also throttled, but no new timer is set since + # there's already one. + self.vm.hmp_qemu_io("drive0", "aio_read 0 2048") + + # Read from drive1. This request is also throttled, and no + # timer is set in drive1 because there's already one in + # drive0. self.vm.hmp_qemu_io("drive1", "aio_read 0 4096") # At this point only the first 4KB have been read from drive0. @@ -248,7 +251,7 @@ class ThrottleTestCase(iotests.QMPTestCase): result = self.vm.qmp("block_set_io_throttle", conv_keys=False, **params) self.assert_qmp(result, 'return', {}) - # Removing the I/O limits from drive0 drains its pending request. + # Removing the I/O limits from drive0 drains its two pending requests. # The read request in drive1 is still throttled. self.assertEqual(self.blockstats('drive0')[0], 8192) self.assertEqual(self.blockstats('drive1')[0], 0) From 25b8e4db7f3baca47244ccd945eb62b32a6e2019 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Thu, 2 Aug 2018 17:50:26 +0300 Subject: [PATCH 05/21] throttle-groups: Don't allow timers without throttled requests Commit 6fccbb475bc6effc313ee9481726a1748b6dae57 fixed a bug caused by QEMU attempting to remove a throttle group member with no pending requests but an active timer set. This was the result of a previous bdrv_drained_begin() call processing the throttled requests but leaving the timer untouched. Although the commit does solve the problem, the situation shouldn't happen in the first place. If we try to drain a throttle group member which has a timer set, we should cancel the timer instead of ignoring it. Signed-off-by: Alberto Garcia Signed-off-by: Kevin Wolf --- block/throttle-groups.c | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/block/throttle-groups.c b/block/throttle-groups.c index d46c56b31e..5d8213a443 100644 --- a/block/throttle-groups.c +++ b/block/throttle-groups.c @@ -36,6 +36,7 @@ static void throttle_group_obj_init(Object *obj); static void throttle_group_obj_complete(UserCreatable *obj, Error **errp); +static void timer_cb(ThrottleGroupMember *tgm, bool is_write); /* The ThrottleGroup structure (with its ThrottleState) is shared * among different ThrottleGroupMembers and it's independent from @@ -424,15 +425,31 @@ static void throttle_group_restart_queue(ThrottleGroupMember *tgm, bool is_write rd->tgm = tgm; rd->is_write = is_write; + /* This function is called when a timer is fired or when + * throttle_group_restart_tgm() is called. Either way, there can + * be no timer pending on this tgm at this point */ + assert(!timer_pending(tgm->throttle_timers.timers[is_write])); + co = qemu_coroutine_create(throttle_group_restart_queue_entry, rd); aio_co_enter(tgm->aio_context, co); } void throttle_group_restart_tgm(ThrottleGroupMember *tgm) { + int i; + if (tgm->throttle_state) { - throttle_group_restart_queue(tgm, 0); - throttle_group_restart_queue(tgm, 1); + for (i = 0; i < 2; i++) { + QEMUTimer *t = tgm->throttle_timers.timers[i]; + if (timer_pending(t)) { + /* If there's a pending timer on this tgm, fire it now */ + timer_del(t); + timer_cb(tgm, i); + } else { + /* Else run the next request from the queue manually */ + throttle_group_restart_queue(tgm, i); + } + } } } @@ -567,16 +584,11 @@ void throttle_group_unregister_tgm(ThrottleGroupMember *tgm) return; } - assert(tgm->pending_reqs[0] == 0 && tgm->pending_reqs[1] == 0); - assert(qemu_co_queue_empty(&tgm->throttled_reqs[0])); - assert(qemu_co_queue_empty(&tgm->throttled_reqs[1])); - qemu_mutex_lock(&tg->lock); for (i = 0; i < 2; i++) { - if (timer_pending(tgm->throttle_timers.timers[i])) { - tg->any_timer_armed[i] = false; - schedule_next_request(tgm, i); - } + assert(tgm->pending_reqs[i] == 0); + assert(qemu_co_queue_empty(&tgm->throttled_reqs[i])); + assert(!timer_pending(tgm->throttle_timers.timers[i])); if (tg->tokens[i] == tgm) { token = throttle_group_next_tgm(tgm); /* Take care of the case where this is the last tgm in the group */ From 497da8236ab2663a8108858ba7ea59aac21c5fe6 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 14 Aug 2018 15:25:51 +0800 Subject: [PATCH 06/21] luks: Allow share-rw=on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Format drivers such as qcow2 don't allow sharing the same image between two QEMU instances in order to prevent image corruptions, because of metadata cache. LUKS driver don't modify metadata except for when creating image, so it is safe to relax the permission. This makes share-rw=on property work on virtual devices. Suggested-by: Daniel P. Berrangé Signed-off-by: Fam Zheng Reviewed-by: Daniel P. Berrangé Signed-off-by: Kevin Wolf --- block/crypto.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/block/crypto.c b/block/crypto.c index 146d81c90a..33ee01bebd 100644 --- a/block/crypto.c +++ b/block/crypto.c @@ -627,7 +627,9 @@ BlockDriver bdrv_crypto_luks = { .bdrv_probe = block_crypto_probe_luks, .bdrv_open = block_crypto_open_luks, .bdrv_close = block_crypto_close, - .bdrv_child_perm = bdrv_format_default_perms, + /* This driver doesn't modify LUKS metadata except when creating image. + * Allow share-rw=on as a special case. */ + .bdrv_child_perm = bdrv_filter_default_perms, .bdrv_co_create = block_crypto_co_create_luks, .bdrv_co_create_opts = block_crypto_co_create_opts_luks, .bdrv_co_truncate = block_crypto_co_truncate, From b24ec3c46281286a6a5624d0ceb5fa6f30bd8a4f Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 13 Jun 2018 11:01:30 +0200 Subject: [PATCH 07/21] block: Remove deprecated -drive geometry options This reinstates commit a7aff6dd10b16b67e8b142d0c94c5d92c3fe88f6, which was temporarily reverted for the 3.0 release so that libvirt gets some extra time to update their command lines. The -drive options cyls, heads, secs and trans were deprecated in QEMU 2.10. It's time to remove them. hd-geo-test tested both the old version with geometry options in -drive and the new one with -device. Therefore the code using -drive doesn't have to be replaced there, we just need to remove the -drive test cases. This in turn allows some simplification of the code. Signed-off-by: Kevin Wolf Reviewed-by: Markus Armbruster --- blockdev.c | 75 +-------------------------------------- hmp-commands.hx | 1 - hw/block/block.c | 14 -------- include/sysemu/blockdev.h | 1 - qemu-deprecated.texi | 5 --- qemu-options.hx | 7 +--- tests/hd-geo-test.c | 37 ++++--------------- 7 files changed, 9 insertions(+), 131 deletions(-) diff --git a/blockdev.c b/blockdev.c index dcf8c8d2ab..c23587b075 100644 --- a/blockdev.c +++ b/blockdev.c @@ -730,22 +730,6 @@ QemuOptsList qemu_legacy_drive_opts = { .name = "if", .type = QEMU_OPT_STRING, .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)", - },{ - .name = "cyls", - .type = QEMU_OPT_NUMBER, - .help = "number of cylinders (ide disk geometry)", - },{ - .name = "heads", - .type = QEMU_OPT_NUMBER, - .help = "number of heads (ide disk geometry)", - },{ - .name = "secs", - .type = QEMU_OPT_NUMBER, - .help = "number of sectors (ide disk geometry)", - },{ - .name = "trans", - .type = QEMU_OPT_STRING, - .help = "chs translation (auto, lba, none)", },{ .name = "addr", .type = QEMU_OPT_STRING, @@ -792,7 +776,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) QemuOpts *legacy_opts; DriveMediaType media = MEDIA_DISK; BlockInterfaceType type; - int cyls, heads, secs, translation; int max_devs, bus_id, unit_id, index; const char *devaddr; const char *werror, *rerror; @@ -803,7 +786,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) Error *local_err = NULL; int i; const char *deprecated[] = { - "serial", "trans", "secs", "heads", "cyls", "addr" + "serial", "addr" }; /* Change legacy command line options into QMP ones */ @@ -932,57 +915,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) type = block_default_type; } - /* Geometry */ - cyls = qemu_opt_get_number(legacy_opts, "cyls", 0); - heads = qemu_opt_get_number(legacy_opts, "heads", 0); - secs = qemu_opt_get_number(legacy_opts, "secs", 0); - - if (cyls || heads || secs) { - if (cyls < 1) { - error_report("invalid physical cyls number"); - goto fail; - } - if (heads < 1) { - error_report("invalid physical heads number"); - goto fail; - } - if (secs < 1) { - error_report("invalid physical secs number"); - goto fail; - } - } - - translation = BIOS_ATA_TRANSLATION_AUTO; - value = qemu_opt_get(legacy_opts, "trans"); - if (value != NULL) { - if (!cyls) { - error_report("'%s' trans must be used with cyls, heads and secs", - value); - goto fail; - } - if (!strcmp(value, "none")) { - translation = BIOS_ATA_TRANSLATION_NONE; - } else if (!strcmp(value, "lba")) { - translation = BIOS_ATA_TRANSLATION_LBA; - } else if (!strcmp(value, "large")) { - translation = BIOS_ATA_TRANSLATION_LARGE; - } else if (!strcmp(value, "rechs")) { - translation = BIOS_ATA_TRANSLATION_RECHS; - } else if (!strcmp(value, "auto")) { - translation = BIOS_ATA_TRANSLATION_AUTO; - } else { - error_report("'%s' invalid translation type", value); - goto fail; - } - } - - if (media == MEDIA_CDROM) { - if (cyls || secs || heads) { - error_report("CHS can't be set with media=cdrom"); - goto fail; - } - } - /* Device address specified by bus/unit or index. * If none was specified, try to find the first free one. */ bus_id = qemu_opt_get_number(legacy_opts, "bus", 0); @@ -1105,11 +1037,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) dinfo = g_malloc0(sizeof(*dinfo)); dinfo->opts = all_opts; - dinfo->cyls = cyls; - dinfo->heads = heads; - dinfo->secs = secs; - dinfo->trans = translation; - dinfo->type = type; dinfo->bus = bus_id; dinfo->unit = unit_id; diff --git a/hmp-commands.hx b/hmp-commands.hx index 91dfe51c37..c1fc747403 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1306,7 +1306,6 @@ ETEXI .params = "[-n] [[:]:]\n" "[file=file][,if=type][,bus=n]\n" "[,unit=m][,media=d][,index=i]\n" - "[,cyls=c,heads=h,secs=s[,trans=t]]\n" "[,snapshot=on|off][,cache=on|off]\n" "[,readonly=on|off][,copy-on-read=on|off]", .help = "add drive to PCI storage controller", diff --git a/hw/block/block.c b/hw/block/block.c index b91e2b6d7e..b6c80ab0b7 100644 --- a/hw/block/block.c +++ b/hw/block/block.c @@ -108,20 +108,6 @@ bool blkconf_geometry(BlockConf *conf, int *ptrans, unsigned cyls_max, unsigned heads_max, unsigned secs_max, Error **errp) { - DriveInfo *dinfo; - - if (!conf->cyls && !conf->heads && !conf->secs) { - /* try to fall back to value set with legacy -drive cyls=... */ - dinfo = blk_legacy_dinfo(conf->blk); - if (dinfo) { - conf->cyls = dinfo->cyls; - conf->heads = dinfo->heads; - conf->secs = dinfo->secs; - if (ptrans) { - *ptrans = dinfo->trans; - } - } - } if (!conf->cyls && !conf->heads && !conf->secs) { hd_geometry_guess(conf->blk, &conf->cyls, &conf->heads, &conf->secs, diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h index ac22f2ae1f..37ea39719e 100644 --- a/include/sysemu/blockdev.h +++ b/include/sysemu/blockdev.h @@ -35,7 +35,6 @@ struct DriveInfo { int auto_del; /* see blockdev_mark_auto_del() */ bool is_default; /* Added by default_drive() ? */ int media_cd; - int cyls, heads, secs, trans; QemuOpts *opts; char *serial; QTAILQ_ENTRY(DriveInfo) next; diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi index 9920a85adc..f90436da56 100644 --- a/qemu-deprecated.texi +++ b/qemu-deprecated.texi @@ -94,11 +94,6 @@ with ``-device ...,netdev=x''), or ``-nic user,smb=/some/dir'' (for embedded NICs). The new syntax allows different settings to be provided per NIC. -@subsection -drive cyls=...,heads=...,secs=...,trans=... (since 2.10.0) - -The drive geometry arguments are replaced by the the geometry arguments -that can be specified with the ``-device'' parameter. - @subsection -drive serial=... (since 2.10.0) The drive serial argument is replaced by the the serial argument diff --git a/qemu-options.hx b/qemu-options.hx index b1bf0f485f..b5155c925a 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -804,9 +804,8 @@ ETEXI DEF("drive", HAS_ARG, QEMU_OPTION_drive, "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n" - " [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n" " [,cache=writethrough|writeback|none|directsync|unsafe][,format=f]\n" - " [,serial=s][,addr=A][,rerror=ignore|stop|report]\n" + " [,snapshot=on|off][,serial=s][,addr=A][,rerror=ignore|stop|report]\n" " [,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n" " [,readonly=on|off][,copy-on-read=on|off]\n" " [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n" @@ -847,10 +846,6 @@ This option defines where is connected the drive by using an index in the list of available connectors of a given interface type. @item media=@var{media} This option defines the type of the media: disk or cdrom. -@item cyls=@var{c},heads=@var{h},secs=@var{s}[,trans=@var{t}] -Force disk physical geometry and the optional BIOS translation (trans=none or -lba). These parameters are deprecated, use the corresponding parameters -of @code{-device} instead. @item snapshot=@var{snapshot} @var{snapshot} is "on" or "off" and controls snapshot mode for the given drive (see @option{-snapshot}). diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c index 24870b38f4..ce665f1f83 100644 --- a/tests/hd-geo-test.c +++ b/tests/hd-geo-test.c @@ -201,7 +201,7 @@ static void setup_mbr(int img_idx, MBRcontents mbr) static int setup_ide(int argc, char *argv[], int argv_sz, int ide_idx, const char *dev, int img_idx, - MBRcontents mbr, const char *opts) + MBRcontents mbr) { char *s1, *s2, *s3; @@ -216,7 +216,7 @@ static int setup_ide(int argc, char *argv[], int argv_sz, s3 = g_strdup(",media=cdrom"); } argc = append_arg(argc, argv, argv_sz, - g_strdup_printf("%s%s%s%s", s1, s2, s3, opts)); + g_strdup_printf("%s%s%s", s1, s2, s3)); g_free(s1); g_free(s2); g_free(s3); @@ -260,7 +260,7 @@ static void test_ide_mbr(bool use_device, MBRcontents mbr) for (i = 0; i < backend_last; i++) { cur_ide[i] = &hd_chst[i][mbr]; dev = use_device ? (is_hd(cur_ide[i]) ? "ide-hd" : "ide-cd") : NULL; - argc = setup_ide(argc, argv, ARGV_SIZE, i, dev, i, mbr, ""); + argc = setup_ide(argc, argv, ARGV_SIZE, i, dev, i, mbr); } args = g_strjoinv(" ", argv); qtest_start(args); @@ -327,16 +327,12 @@ static void test_ide_drive_user(const char *dev, bool trans) const CHST expected_chst = { secs / (4 * 32) , 4, 32, trans }; argc = setup_common(argv, ARGV_SIZE); - opts = g_strdup_printf("%s,%s%scyls=%d,heads=%d,secs=%d", - dev ?: "", - trans && dev ? "bios-chs-" : "", - trans ? "trans=lba," : "", + opts = g_strdup_printf("%s,%scyls=%d,heads=%d,secs=%d", + dev, trans ? "bios-chs-trans=lba," : "", expected_chst.cyls, expected_chst.heads, expected_chst.secs); cur_ide[0] = &expected_chst; - argc = setup_ide(argc, argv, ARGV_SIZE, - 0, dev ? opts : NULL, backend_small, mbr_chs, - dev ? "" : opts); + argc = setup_ide(argc, argv, ARGV_SIZE, 0, opts, backend_small, mbr_chs); g_free(opts); args = g_strjoinv(" ", argv); qtest_start(args); @@ -346,22 +342,6 @@ static void test_ide_drive_user(const char *dev, bool trans) qtest_end(); } -/* - * Test case: IDE device (if=ide) with explicit CHS - */ -static void test_ide_drive_user_chs(void) -{ - test_ide_drive_user(NULL, false); -} - -/* - * Test case: IDE device (if=ide) with explicit CHS and translation - */ -static void test_ide_drive_user_chst(void) -{ - test_ide_drive_user(NULL, true); -} - /* * Test case: IDE device (if=none) with explicit CHS */ @@ -392,8 +372,7 @@ static void test_ide_drive_cd_0(void) for (i = 0; i <= backend_empty; i++) { ide_idx = backend_empty - i; cur_ide[ide_idx] = &hd_chst[i][mbr_blank]; - argc = setup_ide(argc, argv, ARGV_SIZE, - ide_idx, NULL, i, mbr_blank, ""); + argc = setup_ide(argc, argv, ARGV_SIZE, ide_idx, NULL, i, mbr_blank); } args = g_strjoinv(" ", argv); qtest_start(args); @@ -422,8 +401,6 @@ int main(int argc, char **argv) qtest_add_func("hd-geo/ide/drive/mbr/blank", test_ide_drive_mbr_blank); qtest_add_func("hd-geo/ide/drive/mbr/lba", test_ide_drive_mbr_lba); qtest_add_func("hd-geo/ide/drive/mbr/chs", test_ide_drive_mbr_chs); - qtest_add_func("hd-geo/ide/drive/user/chs", test_ide_drive_user_chs); - qtest_add_func("hd-geo/ide/drive/user/chst", test_ide_drive_user_chst); qtest_add_func("hd-geo/ide/drive/cd_0", test_ide_drive_cd_0); qtest_add_func("hd-geo/ide/device/mbr/blank", test_ide_device_mbr_blank); qtest_add_func("hd-geo/ide/device/mbr/lba", test_ide_device_mbr_lba); From 7f8fc971558219891ced425e8fe7a65d2a8f131f Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 13 Jun 2018 11:01:30 +0200 Subject: [PATCH 08/21] block: Remove deprecated -drive option addr This reinstates commit eae3bd1eb7c6b105d30ec06008b3bc3dfc5f45bb, which was temporarily reverted for the 3.0 release so that libvirt gets some extra time to update their command lines. The -drive option addr was deprecated in QEMU 2.10. It's time to remove it. Signed-off-by: Kevin Wolf Reviewed-by: Markus Armbruster Reviewed-by: Jeff Cody --- blockdev.c | 17 +---------------- device-hotplug.c | 4 ---- include/sysemu/blockdev.h | 1 - qemu-deprecated.texi | 5 ----- qemu-options.hx | 5 +---- 5 files changed, 2 insertions(+), 30 deletions(-) diff --git a/blockdev.c b/blockdev.c index c23587b075..6c530769fd 100644 --- a/blockdev.c +++ b/blockdev.c @@ -730,10 +730,6 @@ QemuOptsList qemu_legacy_drive_opts = { .name = "if", .type = QEMU_OPT_STRING, .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)", - },{ - .name = "addr", - .type = QEMU_OPT_STRING, - .help = "pci address (virtio only)", },{ .name = "serial", .type = QEMU_OPT_STRING, @@ -777,7 +773,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) DriveMediaType media = MEDIA_DISK; BlockInterfaceType type; int max_devs, bus_id, unit_id, index; - const char *devaddr; const char *werror, *rerror; bool read_only = false; bool copy_on_read; @@ -786,7 +781,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) Error *local_err = NULL; int i; const char *deprecated[] = { - "serial", "addr" + "serial" }; /* Change legacy command line options into QMP ones */ @@ -976,12 +971,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) } /* Add virtio block device */ - devaddr = qemu_opt_get(legacy_opts, "addr"); - if (devaddr && type != IF_VIRTIO) { - error_report("addr is not supported by this bus type"); - goto fail; - } - if (type == IF_VIRTIO) { QemuOpts *devopts; devopts = qemu_opts_create(qemu_find_opts("device"), NULL, 0, @@ -993,9 +982,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) } qemu_opt_set(devopts, "drive", qdict_get_str(bs_opts, "id"), &error_abort); - if (devaddr) { - qemu_opt_set(devopts, "addr", devaddr, &error_abort); - } } filename = qemu_opt_get(legacy_opts, "file"); @@ -1040,7 +1026,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) dinfo->type = type; dinfo->bus = bus_id; dinfo->unit = unit_id; - dinfo->devaddr = devaddr; dinfo->serial = g_strdup(serial); blk_set_legacy_dinfo(blk, dinfo); diff --git a/device-hotplug.c b/device-hotplug.c index 23fd6656f1..cd427e2c76 100644 --- a/device-hotplug.c +++ b/device-hotplug.c @@ -69,10 +69,6 @@ void hmp_drive_add(Monitor *mon, const QDict *qdict) if (!dinfo) { goto err; } - if (dinfo->devaddr) { - monitor_printf(mon, "Parameter addr not supported\n"); - goto err; - } switch (dinfo->type) { case IF_NONE: diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h index 37ea39719e..c0ae3700ec 100644 --- a/include/sysemu/blockdev.h +++ b/include/sysemu/blockdev.h @@ -28,7 +28,6 @@ typedef enum { } BlockInterfaceType; struct DriveInfo { - const char *devaddr; BlockInterfaceType type; int bus; int unit; diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi index f90436da56..6d7aaab2cd 100644 --- a/qemu-deprecated.texi +++ b/qemu-deprecated.texi @@ -99,11 +99,6 @@ provided per NIC. The drive serial argument is replaced by the the serial argument that can be specified with the ``-device'' parameter. -@subsection -drive addr=... (since 2.10.0) - -The drive addr argument is replaced by the the addr argument -that can be specified with the ``-device'' parameter. - @subsection -usbdevice (since 2.10.0) The ``-usbdevice DEV'' argument is now a synonym for setting diff --git a/qemu-options.hx b/qemu-options.hx index b5155c925a..a42917f41c 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -805,7 +805,7 @@ ETEXI DEF("drive", HAS_ARG, QEMU_OPTION_drive, "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n" " [,cache=writethrough|writeback|none|directsync|unsafe][,format=f]\n" - " [,snapshot=on|off][,serial=s][,addr=A][,rerror=ignore|stop|report]\n" + " [,snapshot=on|off][,serial=s][,rerror=ignore|stop|report]\n" " [,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n" " [,readonly=on|off][,copy-on-read=on|off]\n" " [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n" @@ -883,9 +883,6 @@ an untrusted format header. This option specifies the serial number to assign to the device. This parameter is deprecated, use the corresponding parameter of @code{-device} instead. -@item addr=@var{addr} -Specify the controller's PCI address (if=virtio only). This parameter is -deprecated, use the corresponding parameter of @code{-device} instead. @item werror=@var{action},rerror=@var{action} Specify which @var{action} to take on write and read errors. Valid actions are: "ignore" (ignore the error and try to continue), "stop" (pause QEMU), From 572023f7b235679716c4e57c9d701d83444f14b8 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 13 Jun 2018 11:01:30 +0200 Subject: [PATCH 09/21] block: Remove deprecated -drive option serial This reinstates commit b0083267444a5e0f28391f6c2831a539f878d424, which was temporarily reverted for the 3.0 release so that libvirt gets some extra time to update their command lines. The -drive option serial was deprecated in QEMU 2.10. It's time to remove it. Tests need to be updated to set the serial number with -global instead of using the -drive option. Signed-off-by: Kevin Wolf Reviewed-by: Markus Armbruster Reviewed-by: Jeff Cody --- block/block-backend.c | 1 - blockdev.c | 10 ---------- hw/block/block.c | 13 ------------- hw/block/nvme.c | 1 - hw/block/virtio-blk.c | 1 - hw/ide/qdev.c | 1 - hw/scsi/scsi-disk.c | 1 - hw/usb/dev-storage.c | 1 - include/hw/block/block.h | 1 - include/sysemu/blockdev.h | 1 - qemu-deprecated.texi | 5 ----- qemu-options.hx | 6 +----- tests/ahci-test.c | 6 +++--- tests/ide-test.c | 8 ++++---- 14 files changed, 8 insertions(+), 48 deletions(-) diff --git a/block/block-backend.c b/block/block-backend.c index f2f75a977d..fa120630be 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -419,7 +419,6 @@ static void drive_info_del(DriveInfo *dinfo) return; } qemu_opts_del(dinfo->opts); - g_free(dinfo->serial); g_free(dinfo); } diff --git a/blockdev.c b/blockdev.c index 6c530769fd..37eb40670b 100644 --- a/blockdev.c +++ b/blockdev.c @@ -730,10 +730,6 @@ QemuOptsList qemu_legacy_drive_opts = { .name = "if", .type = QEMU_OPT_STRING, .help = "interface (ide, scsi, sd, mtd, floppy, pflash, virtio)", - },{ - .name = "serial", - .type = QEMU_OPT_STRING, - .help = "disk serial number", },{ .name = "file", .type = QEMU_OPT_STRING, @@ -776,12 +772,10 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) const char *werror, *rerror; bool read_only = false; bool copy_on_read; - const char *serial; const char *filename; Error *local_err = NULL; int i; const char *deprecated[] = { - "serial" }; /* Change legacy command line options into QMP ones */ @@ -949,9 +943,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) goto fail; } - /* Serial number */ - serial = qemu_opt_get(legacy_opts, "serial"); - /* no id supplied -> create one */ if (qemu_opts_id(all_opts) == NULL) { char *new_id; @@ -1026,7 +1017,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) dinfo->type = type; dinfo->bus = bus_id; dinfo->unit = unit_id; - dinfo->serial = g_strdup(serial); blk_set_legacy_dinfo(blk, dinfo); diff --git a/hw/block/block.c b/hw/block/block.c index b6c80ab0b7..cf0eb826f1 100644 --- a/hw/block/block.c +++ b/hw/block/block.c @@ -15,19 +15,6 @@ #include "qapi/qapi-types-block.h" #include "qemu/error-report.h" -void blkconf_serial(BlockConf *conf, char **serial) -{ - DriveInfo *dinfo; - - if (!*serial) { - /* try to fall back to value set with legacy -drive serial=... */ - dinfo = blk_legacy_dinfo(conf->blk); - if (dinfo) { - *serial = g_strdup(dinfo->serial); - } - } -} - void blkconf_blocksizes(BlockConf *conf) { BlockBackend *blk = conf->blk; diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 5e508ab1b3..fc7dacb816 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -1217,7 +1217,6 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp) return; } - blkconf_serial(&n->conf, &n->serial); if (!n->serial) { error_setg(errp, "serial property not set"); return; diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 50b5c869e3..225fe44b7a 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -935,7 +935,6 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp) return; } - blkconf_serial(&conf->conf, &conf->serial); if (!blkconf_apply_backend_options(&conf->conf, blk_is_read_only(conf->conf.blk), true, errp)) { diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c index f395d24592..573b022e1e 100644 --- a/hw/ide/qdev.c +++ b/hw/ide/qdev.c @@ -188,7 +188,6 @@ static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp) return; } - blkconf_serial(&dev->conf, &dev->serial); if (kind != IDE_CD) { if (!blkconf_geometry(&dev->conf, &dev->chs_trans, 65535, 16, 255, errp)) { diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index 5bb390773b..5ae7baa082 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -2379,7 +2379,6 @@ static void scsi_realize(SCSIDevice *dev, Error **errp) return; } - blkconf_serial(&s->qdev.conf, &s->serial); blkconf_blocksizes(&s->qdev.conf); if (s->qdev.conf.logical_block_size > diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c index 45a9487cdb..cd5551d94f 100644 --- a/hw/usb/dev-storage.c +++ b/hw/usb/dev-storage.c @@ -599,7 +599,6 @@ static void usb_msd_storage_realize(USBDevice *dev, Error **errp) return; } - blkconf_serial(&s->conf, &dev->serial); blkconf_blocksizes(&s->conf); if (!blkconf_apply_backend_options(&s->conf, blk_is_read_only(blk), true, errp)) { diff --git a/include/hw/block/block.h b/include/hw/block/block.h index d4f4dfffab..e9f9e2223f 100644 --- a/include/hw/block/block.h +++ b/include/hw/block/block.h @@ -72,7 +72,6 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) /* Configuration helpers */ -void blkconf_serial(BlockConf *conf, char **serial); bool blkconf_geometry(BlockConf *conf, int *trans, unsigned cyls_max, unsigned heads_max, unsigned secs_max, Error **errp); diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h index c0ae3700ec..24954b94e0 100644 --- a/include/sysemu/blockdev.h +++ b/include/sysemu/blockdev.h @@ -35,7 +35,6 @@ struct DriveInfo { bool is_default; /* Added by default_drive() ? */ int media_cd; QemuOpts *opts; - char *serial; QTAILQ_ENTRY(DriveInfo) next; }; diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi index 6d7aaab2cd..df319cfd82 100644 --- a/qemu-deprecated.texi +++ b/qemu-deprecated.texi @@ -94,11 +94,6 @@ with ``-device ...,netdev=x''), or ``-nic user,smb=/some/dir'' (for embedded NICs). The new syntax allows different settings to be provided per NIC. -@subsection -drive serial=... (since 2.10.0) - -The drive serial argument is replaced by the the serial argument -that can be specified with the ``-device'' parameter. - @subsection -usbdevice (since 2.10.0) The ``-usbdevice DEV'' argument is now a synonym for setting diff --git a/qemu-options.hx b/qemu-options.hx index a42917f41c..ea4edb4938 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -805,7 +805,7 @@ ETEXI DEF("drive", HAS_ARG, QEMU_OPTION_drive, "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n" " [,cache=writethrough|writeback|none|directsync|unsafe][,format=f]\n" - " [,snapshot=on|off][,serial=s][,rerror=ignore|stop|report]\n" + " [,snapshot=on|off][,rerror=ignore|stop|report]\n" " [,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n" " [,readonly=on|off][,copy-on-read=on|off]\n" " [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n" @@ -879,10 +879,6 @@ The default mode is @option{cache=writeback}. Specify which disk @var{format} will be used rather than detecting the format. Can be used to specify format=raw to avoid interpreting an untrusted format header. -@item serial=@var{serial} -This option specifies the serial number to assign to the device. This -parameter is deprecated, use the corresponding parameter of @code{-device} -instead. @item werror=@var{action},rerror=@var{action} Specify which @var{action} to take on write and read errors. Valid actions are: "ignore" (ignore the error and try to continue), "stop" (pause QEMU), diff --git a/tests/ahci-test.c b/tests/ahci-test.c index 1a7b761304..937ed2f910 100644 --- a/tests/ahci-test.c +++ b/tests/ahci-test.c @@ -180,12 +180,12 @@ static AHCIQState *ahci_boot(const char *cli, ...) s = ahci_vboot(cli, ap); va_end(ap); } else { - cli = "-drive if=none,id=drive0,file=%s,cache=writeback,serial=%s" - ",format=%s" + cli = "-drive if=none,id=drive0,file=%s,cache=writeback,format=%s" " -M q35 " "-device ide-hd,drive=drive0 " + "-global ide-hd.serial=%s " "-global ide-hd.ver=%s"; - s = ahci_boot(cli, tmp_path, "testdisk", imgfmt, "version"); + s = ahci_boot(cli, tmp_path, imgfmt, "testdisk", "version"); } return s; diff --git a/tests/ide-test.c b/tests/ide-test.c index 2384c2c3e2..f39431b1a9 100644 --- a/tests/ide-test.c +++ b/tests/ide-test.c @@ -529,8 +529,8 @@ static void test_bmdma_no_busmaster(void) static void test_bmdma_setup(void) { ide_test_start( - "-drive file=%s,if=ide,serial=%s,cache=writeback,format=raw " - "-global ide-hd.ver=%s", + "-drive file=%s,if=ide,cache=writeback,format=raw " + "-global ide-hd.serial=%s -global ide-hd.ver=%s", tmp_path, "testdisk", "version"); qtest_irq_intercept_in(global_qtest, "ioapic"); } @@ -561,8 +561,8 @@ static void test_identify(void) int ret; ide_test_start( - "-drive file=%s,if=ide,serial=%s,cache=writeback,format=raw " - "-global ide-hd.ver=%s", + "-drive file=%s,if=ide,cache=writeback,format=raw " + "-global ide-hd.serial=%s -global ide-hd.ver=%s", tmp_path, "testdisk", "version"); dev = get_pci_device(&bmdma_bar, &ide_bar); From 6984eb8da202ffb7ea91a473435cc1219b33a07b Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 13 Jun 2018 11:01:30 +0200 Subject: [PATCH 10/21] block: Remove dead deprecation warning code This reinstates commit 6266e900b8083945cb766b45c124fb3c42932cb3, which was temporarily reverted for the 3.0 release so that libvirt gets some extra time to update their command lines. We removed all options from the 'deprecated' array, so the code is dead and can be removed as well. Signed-off-by: Kevin Wolf Reviewed-by: Markus Armbruster --- blockdev.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/blockdev.c b/blockdev.c index 37eb40670b..72f5347df5 100644 --- a/blockdev.c +++ b/blockdev.c @@ -775,8 +775,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) const char *filename; Error *local_err = NULL; int i; - const char *deprecated[] = { - }; /* Change legacy command line options into QMP ones */ static const struct { @@ -853,16 +851,6 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) goto fail; } - /* Other deprecated options */ - if (!qtest_enabled()) { - for (i = 0; i < ARRAY_SIZE(deprecated); i++) { - if (qemu_opt_get(legacy_opts, deprecated[i]) != NULL) { - error_report("'%s' is deprecated, please use the corresponding " - "option of '-device' instead", deprecated[i]); - } - } - } - /* Media type */ value = qemu_opt_get(legacy_opts, "media"); if (value) { From dbfdf6cb3625c68d0ea84ace4a786406cf43d4f3 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 6 Aug 2018 14:35:10 +0200 Subject: [PATCH 11/21] qapi/block: Document restrictions for node names blockdev-add fails if an invalid node name is given, so we should document what a valid node name even is. Reported-by: Cong Li Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake Reviewed-by: Cong Li Reviewed-by: Alberto Garcia --- qapi/block-core.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/qapi/block-core.json b/qapi/block-core.json index 5b9084a394..f20efc97f7 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -3572,6 +3572,9 @@ # @driver: block driver name # @node-name: the node name of the new node (Since 2.0). # This option is required on the top level of blockdev-add. +# Valid node names start with an alphabetic character and may +# contain only alphanumeric characters, '-', '.' and '_'. Their +# maximum length is 31 characters. # @discard: discard-related options (default: ignore) # @cache: cache-related options # @read-only: whether the block device should be read-only (default: false). From 86fae10c64d642256cf019e6829929fa0d259c7a Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 14 Aug 2018 11:52:25 +0200 Subject: [PATCH 12/21] mirror: Fail gracefully for source == target blockdev-mirror with the same node for source and target segfaults today: A node is in its own backing chain, so mirror_start_job() decides that this is an active commit. When adding the intermediate nodes with block_job_add_bdrv(), it starts the iteration through the subchain with the backing file of source, though, so it never reaches target and instead runs into NULL at the base. While we could fix that by starting with source itself, there is no point in allowing mirroring a node into itself and I wouldn't be surprised if this caused more problems later. So just check for this scenario and error out. Cc: qemu-stable@nongnu.org Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block/mirror.c | 5 +++++ tests/qemu-iotests/041 | 6 ++++++ tests/qemu-iotests/041.out | 4 ++-- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index b48c3f8cf5..dd5ca02b09 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -1499,6 +1499,11 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs, buf_size = DEFAULT_MIRROR_BUF_SIZE; } + if (bs == target) { + error_setg(errp, "Can't mirror node into itself"); + return; + } + /* In the case of active commit, add dummy driver to provide consistent * reads on the top, while disabling it in the intermediate nodes, and make * the backing chain writable. */ diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 index c20ac7da87..9336ab6ff5 100755 --- a/tests/qemu-iotests/041 +++ b/tests/qemu-iotests/041 @@ -234,6 +234,12 @@ class TestSingleBlockdev(TestSingleDrive): result = self.vm.qmp("blockdev-add", **args) self.assert_qmp(result, 'return', {}) + def test_mirror_to_self(self): + result = self.vm.qmp(self.qmp_cmd, job_id='job0', + device=self.qmp_target, sync='full', + target=self.qmp_target) + self.assert_qmp(result, 'error/class', 'GenericError') + test_large_cluster = None test_image_not_found = None test_small_buffer2 = None diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out index c28b392b87..e071d0b261 100644 --- a/tests/qemu-iotests/041.out +++ b/tests/qemu-iotests/041.out @@ -1,5 +1,5 @@ -..................................................................................... +........................................................................................ ---------------------------------------------------------------------- -Ran 85 tests +Ran 88 tests OK From 8d65a3ccfd5db7f0436e095cd952f5d0c3a873ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Tue, 14 Aug 2018 13:39:47 +0100 Subject: [PATCH 13/21] qemu-img: fix regression copying secrets during convert MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the convert command is creating an output file that needs secrets, we need to ensure those secrets are passed to both the blk_new_open and bdrv_create API calls. This is done by qemu-img extracting all opts matching the name suffix "key-secret". Unfortunately the code doing this was run after the call to bdrv_create(), which meant the QemuOpts it was extracting secrets from was now empty. Previously this worked by luks as a bug meant the "key-secret" parameters were not purged from the QemuOpts. This bug was fixed in commit b76b4f604521e59f857d6177bc55f6f2e41fd392 Author: Kevin Wolf Date: Thu Jan 11 16:18:08 2018 +0100 qcow2: Use visitor for options in qcow2_create() Exposing the latent bug in qemu-img. This fix simply moves the copying of secrets to before the bdrv_create() call. Cc: qemu-stable@nongnu.org Signed-off-by: Daniel P. Berrangé Signed-off-by: Kevin Wolf --- qemu-img.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index 1acddf693c..b12f4cd19b 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -345,21 +345,6 @@ static int img_add_key_secrets(void *opaque, return 0; } -static BlockBackend *img_open_new_file(const char *filename, - QemuOpts *create_opts, - const char *fmt, int flags, - bool writethrough, bool quiet, - bool force_share) -{ - QDict *options = NULL; - - options = qdict_new(); - qemu_opt_foreach(create_opts, img_add_key_secrets, options, &error_abort); - - return img_open_file(filename, options, fmt, flags, writethrough, quiet, - force_share); -} - static BlockBackend *img_open(bool image_opts, const char *filename, @@ -2018,6 +2003,7 @@ static int img_convert(int argc, char **argv) BlockDriverState *out_bs; QemuOpts *opts = NULL, *sn_opts = NULL; QemuOptsList *create_opts = NULL; + QDict *open_opts = NULL; char *options = NULL; Error *local_err = NULL; bool writethrough, src_writethrough, quiet = false, image_opts = false, @@ -2362,6 +2348,16 @@ static int img_convert(int argc, char **argv) } } + /* + * The later open call will need any decryption secrets, and + * bdrv_create() will purge "opts", so extract them now before + * they are lost. + */ + if (!skip_create) { + open_opts = qdict_new(); + qemu_opt_foreach(opts, img_add_key_secrets, open_opts, &error_abort); + } + if (!skip_create) { /* Create the new image */ ret = bdrv_create(drv, out_filename, opts, &local_err); @@ -2388,8 +2384,9 @@ static int img_convert(int argc, char **argv) * That has to wait for bdrv_create to be improved * to allow filenames in option syntax */ - s.target = img_open_new_file(out_filename, opts, out_fmt, - flags, writethrough, quiet, false); + s.target = img_open_file(out_filename, open_opts, out_fmt, + flags, writethrough, quiet, false); + open_opts = NULL; /* blk_new_open will have freed it */ } if (!s.target) { ret = -1; @@ -2464,6 +2461,7 @@ out: qemu_opts_del(opts); qemu_opts_free(create_opts); qemu_opts_del(sn_opts); + qobject_unref(open_opts); blk_unref(s.target); if (s.src) { for (bs_i = 0; bs_i < s.src_num; bs_i++) { From 3c005293c2ace58c796b917a792c34c02c4d6433 Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Tue, 14 Aug 2018 15:43:19 +0300 Subject: [PATCH 14/21] block: make .bdrv_close optional Signed-off-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Kevin Wolf --- block.c | 4 +++- block/snapshot.c | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/block.c b/block.c index 39f373e035..9694018a68 100644 --- a/block.c +++ b/block.c @@ -3349,7 +3349,9 @@ static void bdrv_close(BlockDriverState *bs) bdrv_drain(bs); /* in case flush left pending I/O */ if (bs->drv) { - bs->drv->bdrv_close(bs); + if (bs->drv->bdrv_close) { + bs->drv->bdrv_close(bs); + } bs->drv = NULL; } diff --git a/block/snapshot.c b/block/snapshot.c index f9903bc94e..3218a542df 100644 --- a/block/snapshot.c +++ b/block/snapshot.c @@ -218,7 +218,9 @@ int bdrv_snapshot_goto(BlockDriverState *bs, qobject_unref(file_options); qdict_put_str(options, "file", bdrv_get_node_name(file)); - drv->bdrv_close(bs); + if (drv->bdrv_close) { + drv->bdrv_close(bs); + } bdrv_unref_child(bs, bs->file); bs->file = NULL; From f66b1f0e2795f6ac0646103a2b3e135985f3a80b Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Tue, 14 Aug 2018 15:43:20 +0300 Subject: [PATCH 15/21] block: drop empty .bdrv_close handlers .bdrv_close handler is optional after previous commit, no needs to keep empty functions more. Signed-off-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Kevin Wolf --- block/blkreplay.c | 5 ----- block/commit.c | 5 ----- block/copy-on-read.c | 6 ------ block/mirror.c | 5 ----- block/null.c | 6 ------ block/raw-format.c | 5 ----- 6 files changed, 32 deletions(-) diff --git a/block/blkreplay.c b/block/blkreplay.c index 766150ade6..b5d9efdeca 100755 --- a/block/blkreplay.c +++ b/block/blkreplay.c @@ -43,10 +43,6 @@ fail: return ret; } -static void blkreplay_close(BlockDriverState *bs) -{ -} - static int64_t blkreplay_getlength(BlockDriverState *bs) { return bdrv_getlength(bs->file->bs); @@ -135,7 +131,6 @@ static BlockDriver bdrv_blkreplay = { .instance_size = 0, .bdrv_open = blkreplay_open, - .bdrv_close = blkreplay_close, .bdrv_child_perm = bdrv_filter_default_perms, .bdrv_getlength = blkreplay_getlength, diff --git a/block/commit.c b/block/commit.c index e1814d9693..eb414579bd 100644 --- a/block/commit.c +++ b/block/commit.c @@ -239,10 +239,6 @@ static void bdrv_commit_top_refresh_filename(BlockDriverState *bs, QDict *opts) bs->backing->bs->filename); } -static void bdrv_commit_top_close(BlockDriverState *bs) -{ -} - static void bdrv_commit_top_child_perm(BlockDriverState *bs, BdrvChild *c, const BdrvChildRole *role, BlockReopenQueue *reopen_queue, @@ -260,7 +256,6 @@ static BlockDriver bdrv_commit_top = { .bdrv_co_preadv = bdrv_commit_top_preadv, .bdrv_co_block_status = bdrv_co_block_status_from_backing, .bdrv_refresh_filename = bdrv_commit_top_refresh_filename, - .bdrv_close = bdrv_commit_top_close, .bdrv_child_perm = bdrv_commit_top_child_perm, }; diff --git a/block/copy-on-read.c b/block/copy-on-read.c index a19164f9eb..64dcc424b5 100644 --- a/block/copy-on-read.c +++ b/block/copy-on-read.c @@ -45,11 +45,6 @@ static int cor_open(BlockDriverState *bs, QDict *options, int flags, } -static void cor_close(BlockDriverState *bs) -{ -} - - #define PERM_PASSTHROUGH (BLK_PERM_CONSISTENT_READ \ | BLK_PERM_WRITE \ | BLK_PERM_RESIZE) @@ -143,7 +138,6 @@ BlockDriver bdrv_copy_on_read = { .format_name = "copy-on-read", .bdrv_open = cor_open, - .bdrv_close = cor_close, .bdrv_child_perm = cor_child_perm, .bdrv_getlength = cor_getlength, diff --git a/block/mirror.c b/block/mirror.c index dd5ca02b09..6cc10df5c9 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -1426,10 +1426,6 @@ static void bdrv_mirror_top_refresh_filename(BlockDriverState *bs, QDict *opts) bs->backing->bs->filename); } -static void bdrv_mirror_top_close(BlockDriverState *bs) -{ -} - static void bdrv_mirror_top_child_perm(BlockDriverState *bs, BdrvChild *c, const BdrvChildRole *role, BlockReopenQueue *reopen_queue, @@ -1456,7 +1452,6 @@ static BlockDriver bdrv_mirror_top = { .bdrv_co_flush = bdrv_mirror_top_flush, .bdrv_co_block_status = bdrv_co_block_status_from_backing, .bdrv_refresh_filename = bdrv_mirror_top_refresh_filename, - .bdrv_close = bdrv_mirror_top_close, .bdrv_child_perm = bdrv_mirror_top_child_perm, }; diff --git a/block/null.c b/block/null.c index 5d610fdfba..d442d3e901 100644 --- a/block/null.c +++ b/block/null.c @@ -97,10 +97,6 @@ static int null_file_open(BlockDriverState *bs, QDict *options, int flags, return ret; } -static void null_close(BlockDriverState *bs) -{ -} - static int64_t null_getlength(BlockDriverState *bs) { BDRVNullState *s = bs->opaque; @@ -263,7 +259,6 @@ static BlockDriver bdrv_null_co = { .bdrv_file_open = null_file_open, .bdrv_parse_filename = null_co_parse_filename, - .bdrv_close = null_close, .bdrv_getlength = null_getlength, .bdrv_co_preadv = null_co_preadv, @@ -283,7 +278,6 @@ static BlockDriver bdrv_null_aio = { .bdrv_file_open = null_file_open, .bdrv_parse_filename = null_aio_parse_filename, - .bdrv_close = null_close, .bdrv_getlength = null_getlength, .bdrv_aio_preadv = null_aio_preadv, diff --git a/block/raw-format.c b/block/raw-format.c index 2fd69cdb08..6f6dc99b2c 100644 --- a/block/raw-format.c +++ b/block/raw-format.c @@ -459,10 +459,6 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags, return 0; } -static void raw_close(BlockDriverState *bs) -{ -} - static int raw_probe(const uint8_t *buf, int buf_size, const char *filename) { /* smallest possible positive score so that raw is used if and only if no @@ -543,7 +539,6 @@ BlockDriver bdrv_raw = { .bdrv_reopen_commit = &raw_reopen_commit, .bdrv_reopen_abort = &raw_reopen_abort, .bdrv_open = &raw_open, - .bdrv_close = &raw_close, .bdrv_child_perm = bdrv_filter_default_perms, .bdrv_co_create_opts = &raw_co_create_opts, .bdrv_co_preadv = &raw_co_preadv, From 655b4b67e380891ee3996349213c62088d71395d Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Fri, 29 Jun 2018 14:36:59 +0300 Subject: [PATCH 16/21] qdict: Make qdict_extract_subqdict() accept dst = NULL This function extracts all options from a QDict starting with a certain prefix and puts them in a new QDict. We'll have a couple of cases where we simply want to discard those options instead of copying them, and that's what this patch does. Signed-off-by: Alberto Garcia Signed-off-by: Kevin Wolf --- qobject/block-qdict.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/qobject/block-qdict.c b/qobject/block-qdict.c index 80c653013f..42054cc274 100644 --- a/qobject/block-qdict.c +++ b/qobject/block-qdict.c @@ -158,20 +158,25 @@ void qdict_flatten(QDict *qdict) qdict_flatten_qdict(qdict, qdict, NULL); } -/* extract all the src QDict entries starting by start into dst */ +/* extract all the src QDict entries starting by start into dst. + * If dst is NULL then the entries are simply removed from src. */ void qdict_extract_subqdict(QDict *src, QDict **dst, const char *start) { const QDictEntry *entry, *next; const char *p; - *dst = qdict_new(); + if (dst) { + *dst = qdict_new(); + } entry = qdict_first(src); while (entry != NULL) { next = qdict_next(src, entry); if (strstart(entry->key, start, &p)) { - qdict_put_obj(*dst, p, qobject_ref(entry->value)); + if (dst) { + qdict_put_obj(*dst, p, qobject_ref(entry->value)); + } qdict_del(src, entry->key); } entry = next; From 2f624b80ba67f19edb3f7c5bc27cddc3c844e443 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Fri, 29 Jun 2018 14:37:00 +0300 Subject: [PATCH 17/21] block: Remove children options from bs->{options,explicit_options} When bdrv_open_inherit() opens a BlockDriverState the options QDict can contain options for some of its children, passed in the form of child-name.option=value So while each child is opened with that subset of options, those same options remain stored in the parent BDS, leaving (at least) two copies of each one of them ("child-name.option=value" in the parent and "option=value" in the child). Having the children options stored in the parent is unnecessary and it can easily lead to an inconsistent state: $ qemu-img create -f qcow2 hd0.qcow2 10M $ qemu-img create -f qcow2 -b hd0.qcow2 hd1.qcow2 $ qemu-img create -f qcow2 -b hd1.qcow2 hd2.qcow2 $ $QEMU -drive file=hd2.qcow2,node-name=hd2,backing.node-name=hd1 This opens a chain of images hd0 <- hd1 <- hd2. Now let's remove hd1 using block_stream: (qemu) block_stream hd2 0 hd0.qcow2 After this hd2 contains backing.node-name=hd1, which is no longer correct because hd1 doesn't exist anymore. This patch removes all children options from the parent dictionaries at the end of bdrv_open_inherit() and bdrv_reopen_queue_child(). Signed-off-by: Alberto Garcia Signed-off-by: Kevin Wolf --- block.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/block.c b/block.c index 9694018a68..a8fbab396d 100644 --- a/block.c +++ b/block.c @@ -2584,6 +2584,7 @@ static BlockDriverState *bdrv_open_inherit(const char *filename, BlockBackend *file = NULL; BlockDriverState *bs; BlockDriver *drv = NULL; + BdrvChild *child; const char *drvname; const char *backing; Error *local_err = NULL; @@ -2767,6 +2768,15 @@ static BlockDriverState *bdrv_open_inherit(const char *filename, } } + /* Remove all children options from bs->options and bs->explicit_options */ + QLIST_FOREACH(child, &bs->children, next) { + char *child_key_dot; + child_key_dot = g_strdup_printf("%s.", child->name); + qdict_extract_subqdict(bs->explicit_options, NULL, child_key_dot); + qdict_extract_subqdict(bs->options, NULL, child_key_dot); + g_free(child_key_dot); + } + bdrv_refresh_filename(bs); /* Check if any unknown options were used */ @@ -2976,6 +2986,7 @@ static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue, } child_key_dot = g_strdup_printf("%s.", child->name); + qdict_extract_subqdict(explicit_options, NULL, child_key_dot); qdict_extract_subqdict(options, &new_child_options, child_key_dot); g_free(child_key_dot); From 1bab38e7bd29347aca642c55a1de91ec6680efce Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Fri, 29 Jun 2018 14:37:01 +0300 Subject: [PATCH 18/21] block: Simplify bdrv_reopen_abort() If a bdrv_reopen_multiple() call fails, then the explicit_options QDict has to be deleted for every entry in the reopen queue. This must happen regardless of whether that entry's bdrv_reopen_prepare() call succeeded or not. This patch simplifies the cleanup code a bit. Signed-off-by: Alberto Garcia Signed-off-by: Kevin Wolf --- block.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/block.c b/block.c index a8fbab396d..e82cfa6fe2 100644 --- a/block.c +++ b/block.c @@ -3050,9 +3050,10 @@ int bdrv_reopen_multiple(AioContext *ctx, BlockReopenQueue *bs_queue, Error **er cleanup: QSIMPLEQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) { - if (ret && bs_entry->prepared) { - bdrv_reopen_abort(&bs_entry->state); - } else if (ret) { + if (ret) { + if (bs_entry->prepared) { + bdrv_reopen_abort(&bs_entry->state); + } qobject_unref(bs_entry->state.explicit_options); } qobject_unref(bs_entry->state.options); @@ -3341,8 +3342,6 @@ void bdrv_reopen_abort(BDRVReopenState *reopen_state) drv->bdrv_reopen_abort(reopen_state); } - qobject_unref(reopen_state->explicit_options); - bdrv_abort_perm_update(reopen_state->bs); } From 4c8350fe1729b072473417dc22fa6410a4383127 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Fri, 29 Jun 2018 14:37:02 +0300 Subject: [PATCH 19/21] block: Update bs->options if bdrv_reopen() succeeds If bdrv_reopen() succeeds then bs->explicit_options is updated with the new values, but bs->options never changes. Here's an example: { "execute": "blockdev-add", "arguments": { "driver": "qcow2", "node-name": "hd0", "overlap-check": "all", "file": { "driver": "file", "filename": "hd0.qcow2" } } } After this, both bs->options and bs->explicit_options contain "overlap-check": "all". Now let's change that using qemu-io's reopen command: (qemu) qemu-io hd0 "reopen -o overlap-check=none" After this, bs->explicit_options contains the new value but bs->options still keeps the old one. This patch updates bs->options after a BDS has been successfully reopened. Signed-off-by: Alberto Garcia Signed-off-by: Kevin Wolf --- block.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/block.c b/block.c index e82cfa6fe2..8c95f9893a 100644 --- a/block.c +++ b/block.c @@ -3055,8 +3055,8 @@ cleanup: bdrv_reopen_abort(&bs_entry->state); } qobject_unref(bs_entry->state.explicit_options); + qobject_unref(bs_entry->state.options); } - qobject_unref(bs_entry->state.options); g_free(bs_entry); } g_free(bs_queue); @@ -3156,6 +3156,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, Error *local_err = NULL; BlockDriver *drv; QemuOpts *opts; + QDict *orig_reopen_opts; const char *value; bool read_only; @@ -3163,6 +3164,11 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, assert(reopen_state->bs->drv != NULL); drv = reopen_state->bs->drv; + /* This function and each driver's bdrv_reopen_prepare() remove + * entries from reopen_state->options as they are processed, so + * we need to make a copy of the original QDict. */ + orig_reopen_opts = qdict_clone_shallow(reopen_state->options); + /* Process generic block layer options */ opts = qemu_opts_create(&bdrv_runtime_opts, NULL, 0, &error_abort); qemu_opts_absorb_qdict(opts, reopen_state->options, &local_err); @@ -3269,8 +3275,13 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, ret = 0; + /* Restore the original reopen_state->options QDict */ + qobject_unref(reopen_state->options); + reopen_state->options = qobject_ref(orig_reopen_opts); + error: qemu_opts_del(opts); + qobject_unref(orig_reopen_opts); return ret; } @@ -3300,8 +3311,10 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state) /* set BDS specific flags now */ qobject_unref(bs->explicit_options); + qobject_unref(bs->options); bs->explicit_options = reopen_state->explicit_options; + bs->options = reopen_state->options; bs->open_flags = reopen_state->flags; bs->read_only = !(reopen_state->flags & BDRV_O_RDWR); From 261dbcb18f80dad3720209285e2ddd4e1a0b22c3 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Fri, 29 Jun 2018 14:37:03 +0300 Subject: [PATCH 20/21] block: Simplify append_open_options() This function returns a BDS's driver-specific options, excluding also those from its children. Since we have just removed all children options from bs->options there's no need to do this last step. We allow references to children, though ("backing": "node0"), so those we still have to remove. Signed-off-by: Alberto Garcia Signed-off-by: Kevin Wolf --- block.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/block.c b/block.c index 8c95f9893a..6161dbe3eb 100644 --- a/block.c +++ b/block.c @@ -5150,16 +5150,13 @@ static bool append_open_options(QDict *d, BlockDriverState *bs) QemuOptDesc *desc; BdrvChild *child; bool found_any = false; - const char *p; for (entry = qdict_first(bs->options); entry; entry = qdict_next(bs->options, entry)) { - /* Exclude options for children */ + /* Exclude node-name references to children */ QLIST_FOREACH(child, &bs->children, next) { - if (strstart(qdict_entry_key(entry), child->name, &p) - && (!*p || *p == '.')) - { + if (!strcmp(entry->key, child->name)) { break; } } From b5fc2d306664c0c1c6c5cf8e164ffa7b8892283e Mon Sep 17 00:00:00 2001 From: Peter Krempa Date: Wed, 15 Aug 2018 12:26:31 +0200 Subject: [PATCH 21/21] qapi: block: Remove mentions of error types which were removed Most of the various error classes were removed prior to the 1.2 release. Remove mentions of the error classes which did not make it. Signed-off-by: Peter Krempa Signed-off-by: Kevin Wolf --- qapi/block-core.json | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/qapi/block-core.json b/qapi/block-core.json index f20efc97f7..4c7a37afdc 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1499,11 +1499,8 @@ # autogenerated. (Since: 2.9) # # Returns: Nothing on success -# If commit or stream is already active on this device, DeviceInUse # If @device does not exist, DeviceNotFound -# If image commit is not supported by this device, NotSupported -# If @base or @top is invalid, a generic error is returned -# If @speed is invalid, InvalidParameter +# Any other error returns a GenericError. # # Since: 1.3 #