From 8f3a73bc57ea83e5b3930d14fc596ea51859987a Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 29 Jan 2016 20:49:10 +0100 Subject: block: Add blk_dev_has_tray() Pull out the check whether a block device has a tray from blk_dev_is_tray_open() into its own function so both attributes (whether there is a tray vs. whether that tray is open) can be queried independently. Cc: qemu-stable Signed-off-by: Max Reitz Reviewed-by: Eric Blake Reviewed-by: Alberto Garcia Message-id: 1454096953-31773-2-git-send-email-mreitz@redhat.com --- block/block-backend.c | 10 +++++++++- include/block/block_int.h | 1 + 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/block/block-backend.c b/block/block-backend.c index efd6146..a4208f1 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -459,6 +459,14 @@ bool blk_dev_has_removable_media(BlockBackend *blk) } /* + * Does @blk's attached device model have a tray? + */ +bool blk_dev_has_tray(BlockBackend *blk) +{ + return blk->dev_ops && blk->dev_ops->is_tray_open; +} + +/* * Notify @blk's attached device model of a media eject request. * If @force is true, the medium is about to be yanked out forcefully. */ @@ -474,7 +482,7 @@ void blk_dev_eject_request(BlockBackend *blk, bool force) */ bool blk_dev_is_tray_open(BlockBackend *blk) { - if (blk->dev_ops && blk->dev_ops->is_tray_open) { + if (blk_dev_has_tray(blk)) { return blk->dev_ops->is_tray_open(blk->dev_opaque); } return false; diff --git a/include/block/block_int.h b/include/block/block_int.h index 428fa33..ec31df1 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -695,6 +695,7 @@ void blk_set_bs(BlockBackend *blk, BlockDriverState *bs); void blk_dev_change_media_cb(BlockBackend *blk, bool load); bool blk_dev_has_removable_media(BlockBackend *blk); +bool blk_dev_has_tray(BlockBackend *blk); void blk_dev_eject_request(BlockBackend *blk, bool force); bool blk_dev_is_tray_open(BlockBackend *blk); bool blk_dev_is_medium_locked(BlockBackend *blk); -- cgit v1.1 From 12c7ec87a7d88919b23736176eba3118d1521372 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 29 Jan 2016 20:49:11 +0100 Subject: blockdev: Fix 'change' for slot devices 'change' and related operations did not work when used on guest devices featuring removable media but no actual tray, because blk_dev_is_tray_open() always returned false for them and the blockdev-{insert,remove}-medium commands required it to return true. Fix this by making blockdev-{insert,remove}-medium work on tray-less devices. Also, blockdev-{open,close}-tray are now explicitly no-ops when invoked on such devices, and blk_dev_change_media_cb() is instead called by blockdev-{insert,remove}-medium (for tray-less devices only). Reported-by: Peter Maydell Cc: qemu-stable Signed-off-by: Max Reitz Reviewed-by: Alberto Garcia Message-id: 1454096953-31773-3-git-send-email-mreitz@redhat.com Reviewed-by: Eric Blake --- blockdev.c | 31 +++++++++++++++++++++++++++++-- qapi/block-core.json | 3 +-- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/blockdev.c b/blockdev.c index 07cfe25..1044a6a 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2304,6 +2304,11 @@ void qmp_blockdev_open_tray(const char *device, bool has_force, bool force, return; } + if (!blk_dev_has_tray(blk)) { + /* Ignore this command on tray-less devices */ + return; + } + if (blk_dev_is_tray_open(blk)) { return; } @@ -2334,6 +2339,11 @@ void qmp_blockdev_close_tray(const char *device, Error **errp) return; } + if (!blk_dev_has_tray(blk)) { + /* Ignore this command on tray-less devices */ + return; + } + if (!blk_dev_is_tray_open(blk)) { return; } @@ -2363,7 +2373,7 @@ void qmp_x_blockdev_remove_medium(const char *device, Error **errp) return; } - if (has_device && !blk_dev_is_tray_open(blk)) { + if (has_device && blk_dev_has_tray(blk) && !blk_dev_is_tray_open(blk)) { error_setg(errp, "Tray of device '%s' is not open", device); return; } @@ -2388,6 +2398,14 @@ void qmp_x_blockdev_remove_medium(const char *device, Error **errp) blk_remove_bs(blk); + if (!blk_dev_has_tray(blk)) { + /* For tray-less devices, blockdev-open-tray is a no-op (or may not be + * called at all); therefore, the medium needs to be ejected here. + * Do it after blk_remove_bs() so blk_is_inserted(blk) returns the @load + * value passed here (i.e. false). */ + blk_dev_change_media_cb(blk, false); + } + out: aio_context_release(aio_context); } @@ -2413,7 +2431,7 @@ static void qmp_blockdev_insert_anon_medium(const char *device, return; } - if (has_device && !blk_dev_is_tray_open(blk)) { + if (has_device && blk_dev_has_tray(blk) && !blk_dev_is_tray_open(blk)) { error_setg(errp, "Tray of device '%s' is not open", device); return; } @@ -2426,6 +2444,15 @@ static void qmp_blockdev_insert_anon_medium(const char *device, blk_insert_bs(blk, bs); QTAILQ_INSERT_TAIL(&bdrv_states, bs, device_list); + + if (!blk_dev_has_tray(blk)) { + /* For tray-less devices, blockdev-close-tray is a no-op (or may not be + * called at all); therefore, the medium needs to be pushed into the + * slot here. + * Do it after blk_insert_bs() so blk_is_inserted(blk) returns the @load + * value passed here (i.e. true). */ + blk_dev_change_media_cb(blk, true); + } } void qmp_x_blockdev_insert_medium(const char *device, const char *node_name, diff --git a/qapi/block-core.json b/qapi/block-core.json index 0a915ed..40239bf 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -2098,8 +2098,7 @@ # respond to the eject request # - if the BlockBackend denoted by @device does not have a guest device attached # to it -# - if the guest device does not have an actual tray and is empty, for instance -# for floppy disk drives +# - if the guest device does not have an actual tray # # @device: block device name # -- cgit v1.1 From abb3e55b5b718d6392441f56ba0729a62105ac56 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 29 Jan 2016 20:49:12 +0100 Subject: Revert "hw/block/fdc: Implement tray status" This reverts the changes that commit 2e1280e8ff95b3145bc6262accc9d447718e5318 applied to hw/block/fdc.c; also, an additional case of drv->media_inserted use has crept in since, which is replaced by a call to blk_is_inserted(). That commit changed tests/fdc-test.c, too, because after it, one less TRAY_MOVED event would be emitted when executing 'change' on an empty drive. However, now, no TRAY_MOVED events will be emitted at all, and the tray_open status returned by query-block will always be false, necessitating (different) changes to tests/fdc-test.c and iotest 118, which is why this patch is not a pure revert of said commit. Signed-off-by: Max Reitz Message-id: 1454096953-31773-4-git-send-email-mreitz@redhat.com Reviewed-by: Eric Blake --- hw/block/fdc.c | 23 +++------- tests/fdc-test.c | 2 - tests/qemu-iotests/118 | 117 +++++++++++++++---------------------------------- 3 files changed, 43 insertions(+), 99 deletions(-) diff --git a/hw/block/fdc.c b/hw/block/fdc.c index e3b0e1e..818e8a4 100644 --- a/hw/block/fdc.c +++ b/hw/block/fdc.c @@ -173,7 +173,6 @@ typedef struct FDrive { uint8_t media_changed; /* Is media changed */ uint8_t media_rate; /* Data rate of medium */ - bool media_inserted; /* Is there a medium in the tray */ bool media_validated; /* Have we validated the media? */ } FDrive; @@ -249,7 +248,7 @@ static int fd_seek(FDrive *drv, uint8_t head, uint8_t track, uint8_t sect, #endif drv->head = head; if (drv->track != track) { - if (drv->media_inserted) { + if (drv->blk != NULL && blk_is_inserted(drv->blk)) { drv->media_changed = 0; } ret = 1; @@ -258,7 +257,7 @@ static int fd_seek(FDrive *drv, uint8_t head, uint8_t track, uint8_t sect, drv->sect = sect; } - if (!drv->media_inserted) { + if (drv->blk == NULL || !blk_is_inserted(drv->blk)) { ret = 2; } @@ -288,7 +287,9 @@ static int pick_geometry(FDrive *drv) bool magic = drv->drive == FLOPPY_DRIVE_TYPE_AUTO; /* We can only pick a geometry if we have a diskette. */ - if (!drv->media_inserted || drv->drive == FLOPPY_DRIVE_TYPE_NONE) { + if (!drv->blk || !blk_is_inserted(drv->blk) || + drv->drive == FLOPPY_DRIVE_TYPE_NONE) + { return -1; } @@ -390,7 +391,7 @@ static void fd_revalidate(FDrive *drv) FLOPPY_DPRINTF("revalidate\n"); if (drv->blk != NULL) { drv->ro = blk_is_read_only(drv->blk); - if (!drv->media_inserted) { + if (!blk_is_inserted(drv->blk)) { FLOPPY_DPRINTF("No disk in drive\n"); drv->disk = FLOPPY_DRIVE_TYPE_NONE; } else if (!drv->media_validated) { @@ -793,7 +794,7 @@ static bool fdrive_media_changed_needed(void *opaque) { FDrive *drive = opaque; - return (drive->media_inserted && drive->media_changed != 1); + return (drive->blk != NULL && drive->media_changed != 1); } static const VMStateDescription vmstate_fdrive_media_changed = { @@ -2285,22 +2286,13 @@ static void fdctrl_change_cb(void *opaque, bool load) { FDrive *drive = opaque; - drive->media_inserted = load && drive->blk && blk_is_inserted(drive->blk); - drive->media_changed = 1; drive->media_validated = false; fd_revalidate(drive); } -static bool fdctrl_is_tray_open(void *opaque) -{ - FDrive *drive = opaque; - return !drive->media_inserted; -} - static const BlockDevOps fdctrl_block_ops = { .change_media_cb = fdctrl_change_cb, - .is_tray_open = fdctrl_is_tray_open, }; /* Init functions */ @@ -2327,7 +2319,6 @@ static void fdctrl_connect_drives(FDCtrl *fdctrl, Error **errp) fd_init(drive); if (drive->blk) { blk_set_dev_ops(drive->blk, &fdctrl_block_ops, drive); - drive->media_inserted = blk_is_inserted(drive->blk); pick_drive_type(drive); } fd_revalidate(drive); diff --git a/tests/fdc-test.c b/tests/fdc-test.c index 526d459..dbabf50 100644 --- a/tests/fdc-test.c +++ b/tests/fdc-test.c @@ -304,7 +304,6 @@ static void test_media_insert(void) qmp_discard_response("{'execute':'change', 'arguments':{" " 'device':'floppy0', 'target': %s, 'arg': 'raw' }}", test_image); - qmp_discard_response(""); /* ignore event (open -> close) */ dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); @@ -335,7 +334,6 @@ static void test_media_change(void) * reset the bit. */ qmp_discard_response("{'execute':'eject', 'arguments':{" " 'device':'floppy0' }}"); - qmp_discard_response(""); /* ignore event */ dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); diff --git a/tests/qemu-iotests/118 b/tests/qemu-iotests/118 index 114d0e2..7caa38c 100755 --- a/tests/qemu-iotests/118 +++ b/tests/qemu-iotests/118 @@ -42,6 +42,9 @@ class ChangeBaseClass(iotests.QMPTestCase): self.has_opened = True def wait_for_open(self): + if not self.has_real_tray: + return + timeout = time.clock() + 3 while not self.has_opened and time.clock() < timeout: self.process_events() @@ -49,6 +52,9 @@ class ChangeBaseClass(iotests.QMPTestCase): self.fail('Timeout while waiting for the tray to open') def wait_for_close(self): + if not self.has_real_tray: + return + timeout = time.clock() + 3 while not self.has_closed and time.clock() < timeout: self.process_events() @@ -65,7 +71,8 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass): self.wait_for_close() result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) + if self.has_real_tray: + self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) def test_blockdev_change_medium(self): @@ -78,7 +85,8 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass): self.wait_for_close() result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) + if self.has_real_tray: + self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) def test_eject(self): @@ -88,7 +96,8 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass): self.wait_for_open() result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', True) + if self.has_real_tray: + self.assert_qmp(result, 'return[0]/tray_open', True) self.assert_qmp_absent(result, 'return[0]/inserted') def test_tray_eject_change(self): @@ -98,7 +107,8 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass): self.wait_for_open() result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', True) + if self.has_real_tray: + self.assert_qmp(result, 'return[0]/tray_open', True) self.assert_qmp_absent(result, 'return[0]/inserted') result = self.vm.qmp('blockdev-change-medium', device='drive0', @@ -109,7 +119,8 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass): self.wait_for_close() result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) + if self.has_real_tray: + self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) def test_tray_open_close(self): @@ -119,7 +130,8 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass): self.wait_for_open() result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', True) + if self.has_real_tray: + self.assert_qmp(result, 'return[0]/tray_open', True) if self.was_empty == True: self.assert_qmp_absent(result, 'return[0]/inserted') else: @@ -132,10 +144,8 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass): self.wait_for_close() result = self.vm.qmp('query-block') - if self.has_real_tray or not self.was_empty: + if self.has_real_tray: self.assert_qmp(result, 'return[0]/tray_open', False) - else: - self.assert_qmp(result, 'return[0]/tray_open', True) if self.was_empty == True: self.assert_qmp_absent(result, 'return[0]/inserted') else: @@ -148,20 +158,18 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass): self.wait_for_open() result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', True) + if self.has_real_tray: + self.assert_qmp(result, 'return[0]/tray_open', True) self.assert_qmp_absent(result, 'return[0]/inserted') result = self.vm.qmp('blockdev-close-tray', device='drive0') self.assert_qmp(result, 'return', {}) - if self.has_real_tray: - self.wait_for_close() + self.wait_for_close() result = self.vm.qmp('query-block') if self.has_real_tray: self.assert_qmp(result, 'return[0]/tray_open', False) - else: - self.assert_qmp(result, 'return[0]/tray_open', True) self.assert_qmp_absent(result, 'return[0]/inserted') def test_tray_open_change(self): @@ -171,7 +179,8 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass): self.wait_for_open() result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', True) + if self.has_real_tray: + self.assert_qmp(result, 'return[0]/tray_open', True) if self.was_empty == True: self.assert_qmp_absent(result, 'return[0]/inserted') else: @@ -185,7 +194,8 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass): self.wait_for_close() result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) + if self.has_real_tray: + self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) def test_cycle(self): @@ -202,7 +212,8 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass): self.wait_for_open() result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', True) + if self.has_real_tray: + self.assert_qmp(result, 'return[0]/tray_open', True) if self.was_empty == True: self.assert_qmp_absent(result, 'return[0]/inserted') else: @@ -212,7 +223,8 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass): self.assert_qmp(result, 'return', {}) result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', True) + if self.has_real_tray: + self.assert_qmp(result, 'return[0]/tray_open', True) self.assert_qmp_absent(result, 'return[0]/inserted') result = self.vm.qmp('x-blockdev-insert-medium', device='drive0', @@ -220,7 +232,8 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass): self.assert_qmp(result, 'return', {}) result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', True) + if self.has_real_tray: + self.assert_qmp(result, 'return[0]/tray_open', True) self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) result = self.vm.qmp('blockdev-close-tray', device='drive0') @@ -229,7 +242,8 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass): self.wait_for_close() result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) + if self.has_real_tray: + self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) def test_close_on_closed(self): @@ -239,16 +253,14 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass): self.assertEquals(self.vm.get_qmp_events(wait=False), []) def test_remove_on_closed(self): - if self.has_opened: - # Empty floppy drive + if not self.has_real_tray: return result = self.vm.qmp('x-blockdev-remove-medium', device='drive0') self.assert_qmp(result, 'error/class', 'GenericError') def test_insert_on_closed(self): - if self.has_opened: - # Empty floppy drive + if not self.has_real_tray: return result = self.vm.qmp('blockdev-add', @@ -366,7 +378,6 @@ class TestChangeReadOnly(ChangeBaseClass): self.vm.launch() result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/ro', True) self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) @@ -376,11 +387,7 @@ class TestChangeReadOnly(ChangeBaseClass): read_only_mode='retain') self.assert_qmp(result, 'return', {}) - self.wait_for_open() - self.wait_for_close() - result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/ro', True) self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) @@ -390,7 +397,6 @@ class TestChangeReadOnly(ChangeBaseClass): self.vm.launch() result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/ro', True) self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) @@ -400,11 +406,7 @@ class TestChangeReadOnly(ChangeBaseClass): read_only_mode='retain') self.assert_qmp(result, 'return', {}) - self.wait_for_open() - self.wait_for_close() - result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/ro', True) self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) @@ -414,7 +416,6 @@ class TestChangeReadOnly(ChangeBaseClass): self.vm.launch() result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/ro', False) self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) @@ -427,7 +428,6 @@ class TestChangeReadOnly(ChangeBaseClass): self.assertEquals(self.vm.get_qmp_events(wait=False), []) result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/ro', False) self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) @@ -437,7 +437,6 @@ class TestChangeReadOnly(ChangeBaseClass): self.vm.launch() result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/ro', True) self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) @@ -448,11 +447,7 @@ class TestChangeReadOnly(ChangeBaseClass): read_only_mode='read-write') self.assert_qmp(result, 'return', {}) - self.wait_for_open() - self.wait_for_close() - result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/ro', False) self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) @@ -462,7 +457,6 @@ class TestChangeReadOnly(ChangeBaseClass): self.vm.launch() result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/ro', False) self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) @@ -473,11 +467,7 @@ class TestChangeReadOnly(ChangeBaseClass): read_only_mode='read-only') self.assert_qmp(result, 'return', {}) - self.wait_for_open() - self.wait_for_close() - result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/ro', True) self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) @@ -486,7 +476,6 @@ class TestChangeReadOnly(ChangeBaseClass): self.vm.launch() result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/ro', False) self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) @@ -497,11 +486,7 @@ class TestChangeReadOnly(ChangeBaseClass): read_only_mode='read-only') self.assert_qmp(result, 'return', {}) - self.wait_for_open() - self.wait_for_close() - result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/ro', True) self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) @@ -511,7 +496,6 @@ class TestChangeReadOnly(ChangeBaseClass): self.vm.launch() result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/ro', False) self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) @@ -522,10 +506,7 @@ class TestChangeReadOnly(ChangeBaseClass): read_only_mode='read-write') self.assert_qmp(result, 'error/class', 'GenericError') - self.assertEquals(self.vm.get_qmp_events(wait=False), []) - result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/ro', False) self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) @@ -535,7 +516,6 @@ class TestChangeReadOnly(ChangeBaseClass): self.vm.launch() result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/ro', True) self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) @@ -545,11 +525,7 @@ class TestChangeReadOnly(ChangeBaseClass): read_only_mode='retain') self.assert_qmp(result, 'return', {}) - self.wait_for_open() - self.wait_for_close() - result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/ro', True) self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) @@ -559,7 +535,6 @@ class TestChangeReadOnly(ChangeBaseClass): self.vm.launch() result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/ro', False) self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) @@ -569,10 +544,7 @@ class TestChangeReadOnly(ChangeBaseClass): read_only_mode='retain') self.assert_qmp(result, 'error/class', 'GenericError') - self.assertEquals(self.vm.get_qmp_events(wait=False), []) - result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/ro', False) self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) @@ -582,7 +554,6 @@ class TestChangeReadOnly(ChangeBaseClass): self.vm.launch() result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/ro', False) self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) @@ -594,13 +565,7 @@ class TestChangeReadOnly(ChangeBaseClass): 'driver': 'file'}}) self.assert_qmp(result, 'return', {}) - result = self.vm.qmp('blockdev-open-tray', device='drive0', force=True) - self.assert_qmp(result, 'return', {}) - - self.wait_for_open() - result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', True) self.assert_qmp(result, 'return[0]/inserted/ro', False) self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) @@ -608,7 +573,6 @@ class TestChangeReadOnly(ChangeBaseClass): self.assert_qmp(result, 'return', {}) result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', True) self.assert_qmp_absent(result, 'return[0]/inserted') result = self.vm.qmp('x-blockdev-insert-medium', device='drive0', @@ -616,17 +580,10 @@ class TestChangeReadOnly(ChangeBaseClass): self.assert_qmp(result, 'return', {}) result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', True) self.assert_qmp(result, 'return[0]/inserted/ro', True) self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) - result = self.vm.qmp('blockdev-close-tray', device='drive0') - self.assert_qmp(result, 'return', {}) - - self.wait_for_close() - result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/ro', True) self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img) @@ -648,7 +605,6 @@ class TestBlockJobsAfterCycle(ChangeBaseClass): self.assert_qmp(result, 'return', {}) result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/image/format', 'null-co') # For device-less BBs, calling blockdev-open-tray or blockdev-close-tray @@ -671,7 +627,6 @@ class TestBlockJobsAfterCycle(ChangeBaseClass): self.assert_qmp(result, 'return', {}) result = self.vm.qmp('query-block') - self.assert_qmp(result, 'return[0]/tray_open', False) self.assert_qmp(result, 'return[0]/inserted/image/filename', old_img) def tearDown(self): -- cgit v1.1 From 327032ce74d0d9fbd4e18528d0f124b68bc56cea Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 29 Jan 2016 20:49:13 +0100 Subject: block/qapi: Emit tray_open only if there is a tray Signed-off-by: Max Reitz Reviewed-by: Eric Blake Reviewed-by: Alberto Garcia Message-id: 1454096953-31773-5-git-send-email-mreitz@redhat.com --- block/qapi.c | 2 +- qapi/block-core.json | 4 ++-- tests/qemu-iotests/067.out | 4 ---- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/block/qapi.c b/block/qapi.c index a49c118..bbe0c9d 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -300,7 +300,7 @@ static void bdrv_query_info(BlockBackend *blk, BlockInfo **p_info, info->locked = blk_dev_is_medium_locked(blk); info->removable = blk_dev_has_removable_media(blk); - if (blk_dev_has_removable_media(blk)) { + if (blk_dev_has_tray(blk)) { info->has_tray_open = true; info->tray_open = blk_dev_is_tray_open(blk); } diff --git a/qapi/block-core.json b/qapi/block-core.json index 40239bf..628a41d 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -382,8 +382,8 @@ # @locked: True if the guest has locked this device from having its media # removed # -# @tray_open: #optional True if the device has a tray and it is open -# (only present if removable is true) +# @tray_open: #optional True if the device's tray is open +# (only present if it has a tray) # # @dirty-bitmaps: #optional dirty bitmaps information (only present if the # driver has one or more dirty bitmaps) (Since 2.0) diff --git a/tests/qemu-iotests/067.out b/tests/qemu-iotests/067.out index 27ad56f..ae3fccb 100644 --- a/tests/qemu-iotests/067.out +++ b/tests/qemu-iotests/067.out @@ -169,7 +169,6 @@ Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,if=none,id=disk "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false }, - "tray_open": false, "type": "unknown" } ] @@ -289,7 +288,6 @@ Testing: "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false }, - "tray_open": false, "type": "unknown" } ] @@ -410,7 +408,6 @@ Testing: "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false }, - "tray_open": false, "type": "unknown" } ] @@ -501,7 +498,6 @@ Testing: "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false }, - "tray_open": false, "type": "unknown" } ] -- cgit v1.1 From 3db1d98a20262228373bb973ca62b1ab64b29af4 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Mon, 25 Jan 2016 10:26:23 +0800 Subject: vmdk: Fix converting to streamOptimized Commit d62d9dc4b8 lifted streamOptimized images's version to 3, but we now refuse to open version 3 images read-write. We need to make streamOptimized an exception to allow converting to it. This fixes the accidentally broken iotests case 059 for the same reason. Signed-off-by: Fam Zheng Signed-off-by: Kevin Wolf Signed-off-by: Max Reitz --- block/vmdk.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/block/vmdk.c b/block/vmdk.c index 698679d..4a5850b 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -571,6 +571,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, VmdkExtent *extent; BDRVVmdkState *s = bs->opaque; int64_t l1_backup_offset = 0; + bool compressed; ret = bdrv_pread(file->bs, sizeof(magic), &header, sizeof(header)); if (ret < 0) { @@ -645,6 +646,8 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, header = footer.header; } + compressed = + le16_to_cpu(header.compressAlgorithm) == VMDK4_COMPRESSION_DEFLATE; if (le32_to_cpu(header.version) > 3) { char buf[64]; snprintf(buf, sizeof(buf), "VMDK version %" PRId32, @@ -652,7 +655,8 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, error_setg(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, bdrv_get_device_or_node_name(bs), "vmdk", buf); return -ENOTSUP; - } else if (le32_to_cpu(header.version) == 3 && (flags & BDRV_O_RDWR)) { + } else if (le32_to_cpu(header.version) == 3 && (flags & BDRV_O_RDWR) && + !compressed) { /* VMware KB 2064959 explains that version 3 added support for * persistent changed block tracking (CBT), and backup software can * read it as version=1 if it doesn't care about the changed area -- cgit v1.1 From cc8c46b7c5fffd6b6ca099799be0af224c589603 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 25 Jan 2016 15:42:56 +0100 Subject: iotests: Limit supported formats for 118 Image formats used in test 118 need to support image creation. Reported-by: Markus Armbruster Signed-off-by: Max Reitz Reviewed-by: Markus Armbruster Signed-off-by: Kevin Wolf --- tests/qemu-iotests/118 | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/qemu-iotests/118 b/tests/qemu-iotests/118 index 7caa38c..9e5951f 100755 --- a/tests/qemu-iotests/118 +++ b/tests/qemu-iotests/118 @@ -672,4 +672,6 @@ if __name__ == '__main__': # We need floppy and IDE CD-ROM iotests.notrun('not suitable for this machine type: %s' % iotests.qemu_default_machine) - iotests.main() + # Need to support image creation + iotests.main(supported_fmts=['vpc', 'parallels', 'qcow', 'vdi', 'qcow2', + 'vmdk', 'raw', 'vhdx', 'qed']) -- cgit v1.1 From d3780c2dce2c452759ee9d94f9d824cf14cc3ab8 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 25 Jan 2016 19:41:08 +0100 Subject: nbd: client_close on error in nbd_co_client_start Use client_close() if an error in nbd_co_client_start() occurs instead of manually inlining parts of it. This fixes an assertion error on the server side if nbd_negotiate() fails. Signed-off-by: Max Reitz Acked-by: Paolo Bonzini Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- nbd/server.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/nbd/server.c b/nbd/server.c index 256feaf..7fa7f53 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -1082,8 +1082,7 @@ static coroutine_fn void nbd_co_client_start(void *opaque) nbd_export_get(exp); } if (nbd_negotiate(data)) { - shutdown(client->sock, 2); - client->close(client); + client_close(client); goto out; } qemu_co_mutex_init(&client->send_lock); -- cgit v1.1 From 05d0fce49722dd18803a70f00a2ecb1ae92d98d3 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 25 Jan 2016 19:41:09 +0100 Subject: iotests: Rename filter_nbd to _filter_nbd in 083 In the patch after the next, this function is moved to common.filter. Therefore, its name should be preceded by an underscore to signify its global availability. To keep the code motion patch clean, we cannot rename it in the same patch, so we need to choose some order of renaming vs. motion. It is better to keep a supposedly global function used by only a single test in that test than to keep a supposedly local function in a common* file and use it from a test, so we should rename the function before moving it. Signed-off-by: Max Reitz Reviewed-by: John Snow Reviewed-by: Fam Zheng Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- tests/qemu-iotests/083 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/083 b/tests/qemu-iotests/083 index 566da99..13495bc 100755 --- a/tests/qemu-iotests/083 +++ b/tests/qemu-iotests/083 @@ -49,7 +49,7 @@ wait_for_tcp_port() { done } -filter_nbd() { +_filter_nbd() { # nbd.c error messages contain function names and line numbers that are prone # to change. Message ordering depends on timing between send and receive # callbacks sometimes, making them unreliable. @@ -84,7 +84,7 @@ EOF $PYTHON nbd-fault-injector.py $extra_args "127.0.0.1:$port" "$TEST_DIR/nbd-fault-injector.conf" 2>&1 >/dev/null & wait_for_tcp_port "127\\.0\\.0\\.1:$port" - $QEMU_IO -c "read 0 512" "$nbd_url" 2>&1 | _filter_qemu_io | filter_nbd + $QEMU_IO -c "read 0 512" "$nbd_url" 2>&1 | _filter_qemu_io | _filter_nbd echo } -- cgit v1.1 From d1f9cd70843f9d66948da46744b656babcc964f8 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 25 Jan 2016 19:41:10 +0100 Subject: iotests: Change coding style of _filter_nbd in 083 In order to be able to move _filter_nbd to common.filter in the next patch, its coding style needs to be adapted to that of common.filter. That means, we have to convert tabs to four spaces, adjust the alignment of the last line (done with spaces already, assuming one tab equals eight spaces), fix the line length of the comment, and add a line break before the opening brace. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- tests/qemu-iotests/083 | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/tests/qemu-iotests/083 b/tests/qemu-iotests/083 index 13495bc..36e6de8 100755 --- a/tests/qemu-iotests/083 +++ b/tests/qemu-iotests/083 @@ -49,15 +49,16 @@ wait_for_tcp_port() { done } -_filter_nbd() { - # nbd.c error messages contain function names and line numbers that are prone - # to change. Message ordering depends on timing between send and receive - # callbacks sometimes, making them unreliable. - # - # Filter out the TCP port number since this changes between runs. - sed -e 's#^.*nbd/.*\.c:.*##g' \ - -e 's#nbd:127\.0\.0\.1:[^:]*:#nbd:127\.0\.0\.1:PORT:#g' \ - -e 's#\(exportname=foo\|PORT\): Failed to .*$#\1#' +_filter_nbd() +{ + # nbd.c error messages contain function names and line numbers that are + # prone to change. Message ordering depends on timing between send and + # receive callbacks sometimes, making them unreliable. + # + # Filter out the TCP port number since this changes between runs. + sed -e 's#^.*nbd/.*\.c:.*##g' \ + -e 's#nbd:127\.0\.0\.1:[^:]*:#nbd:127\.0\.0\.1:PORT:#g' \ + -e 's#\(exportname=foo\|PORT\): Failed to .*$#\1#' } check_disconnect() { -- cgit v1.1 From 60d446881deefc280e60c195009f7f07ac50eeac Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 25 Jan 2016 19:41:11 +0100 Subject: iotests: Move _filter_nbd into common.filter _filter_nbd can be useful for other NBD tests, too, therefore it should reside in common.filter. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- tests/qemu-iotests/083 | 12 ------------ tests/qemu-iotests/common.filter | 12 ++++++++++++ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/qemu-iotests/083 b/tests/qemu-iotests/083 index 36e6de8..aa99278 100755 --- a/tests/qemu-iotests/083 +++ b/tests/qemu-iotests/083 @@ -49,18 +49,6 @@ wait_for_tcp_port() { done } -_filter_nbd() -{ - # nbd.c error messages contain function names and line numbers that are - # prone to change. Message ordering depends on timing between send and - # receive callbacks sometimes, making them unreliable. - # - # Filter out the TCP port number since this changes between runs. - sed -e 's#^.*nbd/.*\.c:.*##g' \ - -e 's#nbd:127\.0\.0\.1:[^:]*:#nbd:127\.0\.0\.1:PORT:#g' \ - -e 's#\(exportname=foo\|PORT\): Failed to .*$#\1#' -} - check_disconnect() { event=$1 when=$2 diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter index cfdb633..33ed1e4 100644 --- a/tests/qemu-iotests/common.filter +++ b/tests/qemu-iotests/common.filter @@ -230,5 +230,17 @@ _filter_qemu_img_map() -e 's/Mapped to *//' | _filter_testdir | _filter_imgfmt } +_filter_nbd() +{ + # nbd.c error messages contain function names and line numbers that are + # prone to change. Message ordering depends on timing between send and + # receive callbacks sometimes, making them unreliable. + # + # Filter out the TCP port number since this changes between runs. + sed -e 's#^.*nbd/.*\.c:.*##g' \ + -e 's#nbd:127\.0\.0\.1:[^:]*:#nbd:127\.0\.0\.1:PORT:#g' \ + -e 's#\(exportname=foo\|PORT\): Failed to .*$#\1#' +} + # make sure this script returns success true -- cgit v1.1 From dd170c06777d11ecee2d4f4977398068b9b38f03 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 25 Jan 2016 19:41:12 +0100 Subject: iotests: Make _filter_nbd drop log lines The NBD log lines ("/your/source/dir/nbd/xyz.c:function():line: error") should not be converted to empty lines but removed altogether. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- tests/qemu-iotests/083.out | 10 ---------- tests/qemu-iotests/common.filter | 2 +- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/tests/qemu-iotests/083.out b/tests/qemu-iotests/083.out index 78cc49a..ef3d1e3 100644 --- a/tests/qemu-iotests/083.out +++ b/tests/qemu-iotests/083.out @@ -51,7 +51,6 @@ no file open, try 'help open' === Check disconnect after neg2 === - read failed: Input/output error === Check disconnect 8 neg2 === @@ -66,42 +65,34 @@ no file open, try 'help open' === Check disconnect before request === - read failed: Input/output error === Check disconnect after request === - read failed: Input/output error === Check disconnect before reply === - read failed: Input/output error === Check disconnect after reply === - read failed: Input/output error === Check disconnect 4 reply === - read failed: Input/output error === Check disconnect 8 reply === - read failed: Input/output error === Check disconnect before data === - read failed: Input/output error === Check disconnect after data === - read 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) @@ -132,7 +123,6 @@ no file open, try 'help open' === Check disconnect after neg-classic === - read failed: Input/output error *** done diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter index 33ed1e4..e1bfdb7 100644 --- a/tests/qemu-iotests/common.filter +++ b/tests/qemu-iotests/common.filter @@ -237,7 +237,7 @@ _filter_nbd() # receive callbacks sometimes, making them unreliable. # # Filter out the TCP port number since this changes between runs. - sed -e 's#^.*nbd/.*\.c:.*##g' \ + sed -e '/nbd\/.*\.c:/d' \ -e 's#nbd:127\.0\.0\.1:[^:]*:#nbd:127\.0\.0\.1:PORT:#g' \ -e 's#\(exportname=foo\|PORT\): Failed to .*$#\1#' } -- cgit v1.1 From 4a940d14b3000bee4826f6938e6e0dd67393cf35 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 25 Jan 2016 19:41:13 +0100 Subject: iotests: Make _filter_nbd support more URL types This function should support URLs of the "nbd://" format (without swallowing the export name), and for "nbd:///" URLs it should replace "?socket=$TEST_DIR" by "?socket=TEST_DIR" because putting the Unix socket files into the test directory makes sense. Signed-off-by: Max Reitz Reviewed-by: John Snow Reviewed-by: Fam Zheng Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- tests/qemu-iotests/common.filter | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter index e1bfdb7..84b7434 100644 --- a/tests/qemu-iotests/common.filter +++ b/tests/qemu-iotests/common.filter @@ -238,7 +238,8 @@ _filter_nbd() # # Filter out the TCP port number since this changes between runs. sed -e '/nbd\/.*\.c:/d' \ - -e 's#nbd:127\.0\.0\.1:[^:]*:#nbd:127\.0\.0\.1:PORT:#g' \ + -e 's#nbd:\(//\)\?127\.0\.0\.1:[0-9]*#nbd:\1127.0.0.1:PORT#g' \ + -e "s#?socket=$TEST_DIR#?socket=TEST_DIR#g" \ -e 's#\(exportname=foo\|PORT\): Failed to .*$#\1#' } -- cgit v1.1 From 15cfba693b8a828315618b8df41acfb79017acf3 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 25 Jan 2016 19:41:14 +0100 Subject: iotests: Make redirecting qemu's stderr optional Redirecting qemu's stderr to stdout makes working with the stderr output difficult due to the other file descriptor magic performed in _launch_qemu ("ambiguous redirect"). Add an option which specifies whether stderr should be redirected to stdout or not (allowing for other modes to be added in the future). Signed-off-by: Max Reitz Reviewed-by: Kevin Wolf Reviewed-by: John Snow Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- tests/qemu-iotests/common.qemu | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/common.qemu b/tests/qemu-iotests/common.qemu index 8bf3969..2548a87 100644 --- a/tests/qemu-iotests/common.qemu +++ b/tests/qemu-iotests/common.qemu @@ -129,6 +129,8 @@ function _send_qemu_cmd() # $qemu_comm_method: set this variable to 'monitor' (case insensitive) # to use the QEMU HMP monitor for communication. # Otherwise, the default of QMP is used. +# $keep_stderr: Set this variable to 'y' to keep QEMU's stderr output on stderr. +# If this variable is empty, stderr will be redirected to stdout. # Returns: # $QEMU_HANDLE: set to a handle value to communicate with this QEMU instance. # @@ -151,11 +153,20 @@ function _launch_qemu() mkfifo "${fifo_out}" mkfifo "${fifo_in}" - QEMU_NEED_PID='y'\ - ${QEMU} -nographic -serial none ${comm} -machine accel=qtest "${@}" \ + if [ -z "$keep_stderr" ]; then + QEMU_NEED_PID='y'\ + ${QEMU} -nographic -serial none ${comm} -machine accel=qtest "${@}" \ >"${fifo_out}" \ 2>&1 \ <"${fifo_in}" & + elif [ "$keep_stderr" = "y" ]; then + QEMU_NEED_PID='y'\ + ${QEMU} -nographic -serial none ${comm} -machine accel=qtest "${@}" \ + >"${fifo_out}" \ + <"${fifo_in}" & + else + exit 1 + fi if [[ "${BASH_VERSINFO[0]}" -ge "5" || ("${BASH_VERSINFO[0]}" -ge "4" && "${BASH_VERSINFO[1]}" -ge "1") ]] -- cgit v1.1 From 34250395fe8f31efee5a736ced9c3b8e33e0dfa3 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 25 Jan 2016 19:41:15 +0100 Subject: iotests: Add test for a nonexistent NBD export Trying to connect to a nonexistent NBD export should not crash the server. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- tests/qemu-iotests/143 | 73 ++++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/143.out | 7 +++++ tests/qemu-iotests/group | 1 + 3 files changed, 81 insertions(+) create mode 100755 tests/qemu-iotests/143 create mode 100644 tests/qemu-iotests/143.out diff --git a/tests/qemu-iotests/143 b/tests/qemu-iotests/143 new file mode 100755 index 0000000..6207368 --- /dev/null +++ b/tests/qemu-iotests/143 @@ -0,0 +1,73 @@ +#!/bin/bash +# +# Test case for connecting to a non-existing NBD export name +# +# Copyright (C) 2016 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +# creator +owner=mreitz@redhat.com + +seq="$(basename $0)" +echo "QA output created by $seq" + +here="$PWD" +tmp=/tmp/$$ +status=1 # failure is the default! + +_cleanup() +{ + rm -f "$TEST_DIR/nbd" +} +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter +. ./common.qemu + +_supported_fmt generic +_supported_proto generic +_supported_os Linux + +keep_stderr=y \ +_launch_qemu 2> >(_filter_nbd) + +_send_qemu_cmd $QEMU_HANDLE \ + "{ 'execute': 'qmp_capabilities' }" \ + 'return' + +_send_qemu_cmd $QEMU_HANDLE \ + "{ 'execute': 'nbd-server-start', + 'arguments': { 'addr': { 'type': 'unix', + 'data': { 'path': '$TEST_DIR/nbd' }}}}" \ + 'return' + +# This should just result in a client error, not in the server crashing +$QEMU_IO_PROG -f raw -c quit \ + "nbd+unix:///no_such_export?socket=$TEST_DIR/nbd" 2>&1 \ + | _filter_qemu_io | _filter_nbd + +_send_qemu_cmd $QEMU_HANDLE \ + "{ 'execute': 'quit' }" \ + 'return' + +wait=1 _cleanup_qemu + +# success, all done +echo '*** done' +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/143.out b/tests/qemu-iotests/143.out new file mode 100644 index 0000000..dad2024 --- /dev/null +++ b/tests/qemu-iotests/143.out @@ -0,0 +1,7 @@ +QA output created by 143 +{"return": {}} +{"return": {}} +can't open device nbd+unix:///no_such_export?socket=TEST_DIR/nbd: Failed to read export length +{"return": {}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"} +*** done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index d6e9219..ac6a959 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -142,3 +142,4 @@ 138 rw auto quick 139 rw auto quick 142 auto +143 auto quick -- cgit v1.1 From e43f7f6f46fdf518de1bea6646455d91495a58c3 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Thu, 28 Jan 2016 11:57:13 +0800 Subject: block: Remove unused struct definition BlockFinishData Unused since 94db6d2d3. Signed-off-by: Fam Zheng Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf Signed-off-by: Max Reitz --- blockjob.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/blockjob.c b/blockjob.c index 80adb9d..a692142 100644 --- a/blockjob.c +++ b/blockjob.c @@ -278,14 +278,6 @@ void block_job_iostatus_reset(BlockJob *job) } } -struct BlockFinishData { - BlockJob *job; - BlockCompletionFunc *cb; - void *opaque; - bool cancelled; - int ret; -}; - static int block_job_finish_sync(BlockJob *job, void (*finish)(BlockJob *, Error **errp), Error **errp) -- cgit v1.1 From c5acdc9ab4e6aa9b05e6242114479333b15d496b Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 29 Jan 2016 16:36:01 +0100 Subject: block: Release named dirty bitmaps in bdrv_close() bdrv_delete() is not very happy about deleting BlockDriverStates with dirty bitmaps still attached to them. In the past, we got around that very easily by relying on bdrv_close_all() bypassing bdrv_delete(), and bdrv_close() simply ignoring that condition. We should fix that by releasing all named dirty bitmaps in bdrv_close() (there should not be any unnamed bitmaps left) and moving the assertion from bdrv_delete() there. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block.c | 39 +++++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/block.c b/block.c index 5709d3d..41ab00e 100644 --- a/block.c +++ b/block.c @@ -88,6 +88,8 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename, const BdrvChildRole *child_role, Error **errp); static void bdrv_dirty_bitmap_truncate(BlockDriverState *bs); +static void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs); + /* If non-zero, use only whitelisted block drivers */ static int use_bdrv_whitelist; @@ -2157,6 +2159,9 @@ void bdrv_close(BlockDriverState *bs) notifier_list_notify(&bs->close_notifiers, bs); + bdrv_release_named_dirty_bitmaps(bs); + assert(QLIST_EMPTY(&bs->dirty_bitmaps)); + if (bs->blk) { blk_dev_change_media_cb(bs->blk, false); } @@ -2366,7 +2371,6 @@ static void bdrv_delete(BlockDriverState *bs) assert(!bs->job); assert(bdrv_op_blocker_is_empty(bs)); assert(!bs->refcnt); - assert(QLIST_EMPTY(&bs->dirty_bitmaps)); bdrv_close(bs); @@ -3582,21 +3586,40 @@ static void bdrv_dirty_bitmap_truncate(BlockDriverState *bs) } } -void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap) +static void bdrv_do_release_matching_dirty_bitmap(BlockDriverState *bs, + BdrvDirtyBitmap *bitmap, + bool only_named) { BdrvDirtyBitmap *bm, *next; QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) { - if (bm == bitmap) { + if ((!bitmap || bm == bitmap) && (!only_named || bm->name)) { assert(!bdrv_dirty_bitmap_frozen(bm)); - QLIST_REMOVE(bitmap, list); - hbitmap_free(bitmap->bitmap); - g_free(bitmap->name); - g_free(bitmap); - return; + QLIST_REMOVE(bm, list); + hbitmap_free(bm->bitmap); + g_free(bm->name); + g_free(bm); + + if (bitmap) { + return; + } } } } +void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap) +{ + bdrv_do_release_matching_dirty_bitmap(bs, bitmap, false); +} + +/** + * Release all named dirty bitmaps attached to a BDS (for use in bdrv_close()). + * There must not be any frozen bitmaps attached. + */ +static void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs) +{ + bdrv_do_release_matching_dirty_bitmap(bs, NULL, true); +} + void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap) { assert(!bdrv_dirty_bitmap_frozen(bitmap)); -- cgit v1.1 From 16dee4183acb3755b8d2e76e6466a6fec5f1350e Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 29 Jan 2016 16:36:02 +0100 Subject: iotests: Add test for eject under NBD server This patch adds a test for ejecting the BlockBackend an NBD server is connected to (the NBD server is supposed to stop). Signed-off-by: Max Reitz Reviewed-by: Fam Zheng Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- tests/qemu-iotests/140 | 92 ++++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/140.out | 16 ++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 109 insertions(+) create mode 100755 tests/qemu-iotests/140 create mode 100644 tests/qemu-iotests/140.out diff --git a/tests/qemu-iotests/140 b/tests/qemu-iotests/140 new file mode 100755 index 0000000..f78c317 --- /dev/null +++ b/tests/qemu-iotests/140 @@ -0,0 +1,92 @@ +#!/bin/bash +# +# Test case for ejecting a BB with an NBD server attached to it +# +# Copyright (C) 2016 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +# creator +owner=mreitz@redhat.com + +seq="$(basename $0)" +echo "QA output created by $seq" + +here="$PWD" +tmp=/tmp/$$ +status=1 # failure is the default! + +_cleanup() +{ + _cleanup_test_img + rm -f "$TEST_DIR/nbd" +} +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter +. ./common.qemu + +_supported_fmt generic +_supported_proto file +_supported_os Linux + +_make_test_img 64k + +$QEMU_IO -c 'write -P 42 0 64k' "$TEST_IMG" | _filter_qemu_io + +keep_stderr=y \ +_launch_qemu -drive if=ide,media=cdrom,id=drv,file="$TEST_IMG",format=$IMGFMT \ + 2> >(_filter_nbd) + +_send_qemu_cmd $QEMU_HANDLE \ + "{ 'execute': 'qmp_capabilities' }" \ + 'return' + +_send_qemu_cmd $QEMU_HANDLE \ + "{ 'execute': 'nbd-server-start', + 'arguments': { 'addr': { 'type': 'unix', + 'data': { 'path': '$TEST_DIR/nbd' }}}}" \ + 'return' + +_send_qemu_cmd $QEMU_HANDLE \ + "{ 'execute': 'nbd-server-add', + 'arguments': { 'device': 'drv' }}" \ + 'return' + +$QEMU_IO_PROG -f raw -c 'read -P 42 0 64k' \ + "nbd+unix:///drv?socket=$TEST_DIR/nbd" 2>&1 \ + | _filter_qemu_io | _filter_nbd + +_send_qemu_cmd $QEMU_HANDLE \ + "{ 'execute': 'eject', + 'arguments': { 'device': 'drv' }}" \ + 'return' + +$QEMU_IO_PROG -f raw -c close \ + "nbd+unix:///drv?socket=$TEST_DIR/nbd" 2>&1 \ + | _filter_qemu_io | _filter_nbd + +_send_qemu_cmd $QEMU_HANDLE \ + "{ 'execute': 'quit' }" \ + 'return' + +wait=1 _cleanup_qemu + +# success, all done +echo '*** done' +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/140.out b/tests/qemu-iotests/140.out new file mode 100644 index 0000000..fdedeb3 --- /dev/null +++ b/tests/qemu-iotests/140.out @@ -0,0 +1,16 @@ +QA output created by 140 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=65536 +wrote 65536/65536 bytes at offset 0 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +{"return": {}} +{"return": {}} +{"return": {}} +read 65536/65536 bytes at offset 0 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "drv", "tray-open": true}} +{"return": {}} +can't open device nbd+unix:///drv?socket=TEST_DIR/nbd: Failed to read export length +no file open, try 'help open' +{"return": {}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"} +*** done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index ac6a959..ff1ff0d 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -141,5 +141,6 @@ 137 rw auto 138 rw auto quick 139 rw auto quick +140 rw auto quick 142 auto 143 auto quick -- cgit v1.1 From 3301f6c6e9b52e370b07b3a08fd11735d0f0f292 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 29 Jan 2016 16:36:03 +0100 Subject: block: Add BB-BDS remove/insert notifiers bdrv_close() no longer signifies ejection of a medium, this is now done by removing the BDS from the BB. Therefore, we want to have a notifier for that in the BB instead of a close notifier in the BDS. The former is added now, the latter is removed later. Symmetrically, another notifier list is added that is invoked whenever a BDS is inserted. We will need that for virtio-blk and virtio-scsi, which can then remove their op blockers on BDS ejection and set them up on insertion. Signed-off-by: Max Reitz Reviewed-by: Kevin Wolf Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/block-backend.c | 20 ++++++++++++++++++++ include/sysemu/block-backend.h | 2 ++ 2 files changed, 22 insertions(+) diff --git a/block/block-backend.c b/block/block-backend.c index a4208f1..1872191 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -49,6 +49,8 @@ struct BlockBackend { BlockdevOnError on_read_error, on_write_error; bool iostatus_enabled; BlockDeviceIoStatus iostatus; + + NotifierList remove_bs_notifiers, insert_bs_notifiers; }; typedef struct BlockBackendAIOCB { @@ -99,6 +101,8 @@ BlockBackend *blk_new(const char *name, Error **errp) blk = g_new0(BlockBackend, 1); blk->name = g_strdup(name); blk->refcnt = 1; + notifier_list_init(&blk->remove_bs_notifiers); + notifier_list_init(&blk->insert_bs_notifiers); QTAILQ_INSERT_TAIL(&blk_backends, blk, link); return blk; } @@ -167,6 +171,8 @@ static void blk_delete(BlockBackend *blk) bdrv_unref(blk->bs); blk->bs = NULL; } + assert(QLIST_EMPTY(&blk->remove_bs_notifiers.notifiers)); + assert(QLIST_EMPTY(&blk->insert_bs_notifiers.notifiers)); if (blk->root_state.throttle_state) { g_free(blk->root_state.throttle_group); throttle_group_unref(blk->root_state.throttle_state); @@ -345,6 +351,8 @@ void blk_hide_on_behalf_of_hmp_drive_del(BlockBackend *blk) */ void blk_remove_bs(BlockBackend *blk) { + notifier_list_notify(&blk->remove_bs_notifiers, blk); + blk_update_root_state(blk); blk->bs->blk = NULL; @@ -361,6 +369,8 @@ void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs) bdrv_ref(bs); blk->bs = bs; bs->blk = blk; + + notifier_list_notify(&blk->insert_bs_notifiers, blk); } /* @@ -1126,6 +1136,16 @@ void blk_remove_aio_context_notifier(BlockBackend *blk, } } +void blk_add_remove_bs_notifier(BlockBackend *blk, Notifier *notify) +{ + notifier_list_add(&blk->remove_bs_notifiers, notify); +} + +void blk_add_insert_bs_notifier(BlockBackend *blk, Notifier *notify) +{ + notifier_list_add(&blk->insert_bs_notifiers, notify); +} + void blk_add_close_notifier(BlockBackend *blk, Notifier *notify) { if (blk->bs) { diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index 1568554..e12be67 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -164,6 +164,8 @@ void blk_remove_aio_context_notifier(BlockBackend *blk, void *), void (*detach_aio_context)(void *), void *opaque); +void blk_add_remove_bs_notifier(BlockBackend *blk, Notifier *notify); +void blk_add_insert_bs_notifier(BlockBackend *blk, Notifier *notify); void blk_add_close_notifier(BlockBackend *blk, Notifier *notify); void blk_io_plug(BlockBackend *blk); void blk_io_unplug(BlockBackend *blk); -- cgit v1.1 From 1b1e0659a4c8888fba559e8d41051339e0a3cd8a Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 29 Jan 2016 16:36:04 +0100 Subject: virtio-blk: Functions for op blocker management Put the code for setting up and removing op blockers into an own function, respectively. Then, we can invoke those functions whenever a BDS is removed from an virtio-blk BB or inserted into it. Signed-off-by: Max Reitz Reviewed-by: Fam Zheng Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- hw/block/dataplane/virtio-blk.c | 77 +++++++++++++++++++++++++++++++---------- 1 file changed, 59 insertions(+), 18 deletions(-) diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index bc34046..ee0c4d4 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -40,6 +40,8 @@ struct VirtIOBlockDataPlane { EventNotifier *guest_notifier; /* irq */ QEMUBH *bh; /* bh for guest notification */ + Notifier insert_notifier, remove_notifier; + /* Note that these EventNotifiers are assigned by value. This is * fine as long as you do not call event_notifier_cleanup on them * (because you don't own the file descriptor or handle; you just @@ -137,6 +139,54 @@ static void handle_notify(EventNotifier *e) blk_io_unplug(s->conf->conf.blk); } +static void data_plane_set_up_op_blockers(VirtIOBlockDataPlane *s) +{ + assert(!s->blocker); + error_setg(&s->blocker, "block device is in use by data plane"); + blk_op_block_all(s->conf->conf.blk, s->blocker); + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_RESIZE, s->blocker); + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_DRIVE_DEL, s->blocker); + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_BACKUP_SOURCE, s->blocker); + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_CHANGE, s->blocker); + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_COMMIT_SOURCE, s->blocker); + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_COMMIT_TARGET, s->blocker); + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_EJECT, s->blocker); + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT, + s->blocker); + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT, + s->blocker); + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT_DELETE, + s->blocker); + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_MIRROR_SOURCE, s->blocker); + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_STREAM, s->blocker); + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_REPLACE, s->blocker); +} + +static void data_plane_remove_op_blockers(VirtIOBlockDataPlane *s) +{ + if (s->blocker) { + blk_op_unblock_all(s->conf->conf.blk, s->blocker); + error_free(s->blocker); + s->blocker = NULL; + } +} + +static void data_plane_blk_insert_notifier(Notifier *n, void *data) +{ + VirtIOBlockDataPlane *s = container_of(n, VirtIOBlockDataPlane, + insert_notifier); + assert(s->conf->conf.blk == data); + data_plane_set_up_op_blockers(s); +} + +static void data_plane_blk_remove_notifier(Notifier *n, void *data) +{ + VirtIOBlockDataPlane *s = container_of(n, VirtIOBlockDataPlane, + remove_notifier); + assert(s->conf->conf.blk == data); + data_plane_remove_op_blockers(s); +} + /* Context: QEMU global mutex held */ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf, VirtIOBlockDataPlane **dataplane, @@ -179,22 +229,12 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf, s->ctx = iothread_get_aio_context(s->iothread); s->bh = aio_bh_new(s->ctx, notify_guest_bh, s); - error_setg(&s->blocker, "block device is in use by data plane"); - blk_op_block_all(conf->conf.blk, s->blocker); - blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_RESIZE, s->blocker); - blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_DRIVE_DEL, s->blocker); - blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_BACKUP_SOURCE, s->blocker); - blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_CHANGE, s->blocker); - blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_COMMIT_SOURCE, s->blocker); - blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_COMMIT_TARGET, s->blocker); - blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_EJECT, s->blocker); - blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT, s->blocker); - blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT, s->blocker); - blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT_DELETE, - s->blocker); - blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_MIRROR_SOURCE, s->blocker); - blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_STREAM, s->blocker); - blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_REPLACE, s->blocker); + s->insert_notifier.notify = data_plane_blk_insert_notifier; + s->remove_notifier.notify = data_plane_blk_remove_notifier; + blk_add_insert_bs_notifier(conf->conf.blk, &s->insert_notifier); + blk_add_remove_bs_notifier(conf->conf.blk, &s->remove_notifier); + + data_plane_set_up_op_blockers(s); *dataplane = s; } @@ -207,8 +247,9 @@ void virtio_blk_data_plane_destroy(VirtIOBlockDataPlane *s) } virtio_blk_data_plane_stop(s); - blk_op_unblock_all(s->conf->conf.blk, s->blocker); - error_free(s->blocker); + data_plane_remove_op_blockers(s); + notifier_remove(&s->insert_notifier); + notifier_remove(&s->remove_notifier); qemu_bh_delete(s->bh); object_unref(OBJECT(s->iothread)); g_free(s); -- cgit v1.1 From 5b9e0e46935538bf2b24fcb70b65477f758413fd Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 29 Jan 2016 16:36:05 +0100 Subject: virtio-scsi: Catch BDS-BB removal/insertion Make use of the BDS-BB removal and insertion notifiers to remove or set up, respectively, virtio-scsi's op blockers. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- hw/scsi/virtio-scsi.c | 55 +++++++++++++++++++++++++++++++++++++++++ include/hw/virtio/virtio-scsi.h | 10 ++++++++ 2 files changed, 65 insertions(+) diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 5f23ab2..1500c42 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -758,6 +758,22 @@ static void virtio_scsi_change(SCSIBus *bus, SCSIDevice *dev, SCSISense sense) } } +static void virtio_scsi_blk_insert_notifier(Notifier *n, void *data) +{ + VirtIOSCSIBlkChangeNotifier *cn = DO_UPCAST(VirtIOSCSIBlkChangeNotifier, + n, n); + assert(cn->sd->conf.blk == data); + blk_op_block_all(cn->sd->conf.blk, cn->s->blocker); +} + +static void virtio_scsi_blk_remove_notifier(Notifier *n, void *data) +{ + VirtIOSCSIBlkChangeNotifier *cn = DO_UPCAST(VirtIOSCSIBlkChangeNotifier, + n, n); + assert(cn->sd->conf.blk == data); + blk_op_unblock_all(cn->sd->conf.blk, cn->s->blocker); +} + static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp) { @@ -766,6 +782,8 @@ static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev, SCSIDevice *sd = SCSI_DEVICE(dev); if (s->ctx && !s->dataplane_disabled) { + VirtIOSCSIBlkChangeNotifier *insert_notifier, *remove_notifier; + if (blk_op_is_blocked(sd->conf.blk, BLOCK_OP_TYPE_DATAPLANE, errp)) { return; } @@ -773,6 +791,20 @@ static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev, aio_context_acquire(s->ctx); blk_set_aio_context(sd->conf.blk, s->ctx); aio_context_release(s->ctx); + + insert_notifier = g_new0(VirtIOSCSIBlkChangeNotifier, 1); + insert_notifier->n.notify = virtio_scsi_blk_insert_notifier; + insert_notifier->s = s; + insert_notifier->sd = sd; + blk_add_insert_bs_notifier(sd->conf.blk, &insert_notifier->n); + QTAILQ_INSERT_TAIL(&s->insert_notifiers, insert_notifier, next); + + remove_notifier = g_new0(VirtIOSCSIBlkChangeNotifier, 1); + remove_notifier->n.notify = virtio_scsi_blk_remove_notifier; + remove_notifier->s = s; + remove_notifier->sd = sd; + blk_add_remove_bs_notifier(sd->conf.blk, &remove_notifier->n); + QTAILQ_INSERT_TAIL(&s->remove_notifiers, remove_notifier, next); } if (virtio_vdev_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) { @@ -788,6 +820,7 @@ static void virtio_scsi_hotunplug(HotplugHandler *hotplug_dev, DeviceState *dev, VirtIODevice *vdev = VIRTIO_DEVICE(hotplug_dev); VirtIOSCSI *s = VIRTIO_SCSI(vdev); SCSIDevice *sd = SCSI_DEVICE(dev); + VirtIOSCSIBlkChangeNotifier *insert_notifier, *remove_notifier; if (virtio_vdev_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) { virtio_scsi_push_event(s, sd, @@ -798,6 +831,25 @@ static void virtio_scsi_hotunplug(HotplugHandler *hotplug_dev, DeviceState *dev, if (s->ctx) { blk_op_unblock_all(sd->conf.blk, s->blocker); } + + QTAILQ_FOREACH(insert_notifier, &s->insert_notifiers, next) { + if (insert_notifier->sd == sd) { + notifier_remove(&insert_notifier->n); + QTAILQ_REMOVE(&s->insert_notifiers, insert_notifier, next); + g_free(insert_notifier); + break; + } + } + + QTAILQ_FOREACH(remove_notifier, &s->remove_notifiers, next) { + if (remove_notifier->sd == sd) { + notifier_remove(&remove_notifier->n); + QTAILQ_REMOVE(&s->remove_notifiers, remove_notifier, next); + g_free(remove_notifier); + break; + } + } + qdev_simple_device_unplug_cb(hotplug_dev, dev, errp); } @@ -912,6 +964,9 @@ static void virtio_scsi_device_realize(DeviceState *dev, Error **errp) add_migration_state_change_notifier(&s->migration_state_notifier); error_setg(&s->blocker, "block device is in use by data plane"); + + QTAILQ_INIT(&s->insert_notifiers); + QTAILQ_INIT(&s->remove_notifiers); } static void virtio_scsi_instance_init(Object *obj) diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h index 088fe9f..0394eb2 100644 --- a/include/hw/virtio/virtio-scsi.h +++ b/include/hw/virtio/virtio-scsi.h @@ -76,6 +76,13 @@ typedef struct VirtIOSCSICommon { VirtQueue **cmd_vqs; } VirtIOSCSICommon; +typedef struct VirtIOSCSIBlkChangeNotifier { + Notifier n; + struct VirtIOSCSI *s; + SCSIDevice *sd; + QTAILQ_ENTRY(VirtIOSCSIBlkChangeNotifier) next; +} VirtIOSCSIBlkChangeNotifier; + typedef struct VirtIOSCSI { VirtIOSCSICommon parent_obj; @@ -86,6 +93,9 @@ typedef struct VirtIOSCSI { /* Fields for dataplane below */ AioContext *ctx; /* one iothread per virtio-scsi-pci for now */ + QTAILQ_HEAD(, VirtIOSCSIBlkChangeNotifier) insert_notifiers; + QTAILQ_HEAD(, VirtIOSCSIBlkChangeNotifier) remove_notifiers; + /* Vring is used instead of vq in dataplane code, because of the underlying * memory layer thread safety */ VirtIOSCSIVring *ctrl_vring; -- cgit v1.1 From 741cc431337891e0c1d69fcba6574aa8df3e4822 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 29 Jan 2016 16:36:06 +0100 Subject: nbd: Switch from close to eject notifier The NBD code uses the BDS close notifier to determine when a medium is ejected. However, now it should use the BB's BDS removal notifier for that instead of the BDS's close notifier. Signed-off-by: Max Reitz Reviewed-by: Fam Zheng Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- blockdev-nbd.c | 40 +++++----------------------------------- nbd/server.c | 13 +++++++++++++ 2 files changed, 18 insertions(+), 35 deletions(-) diff --git a/blockdev-nbd.c b/blockdev-nbd.c index 4a758ac..9d6a21c 100644 --- a/blockdev-nbd.c +++ b/blockdev-nbd.c @@ -45,37 +45,11 @@ void qmp_nbd_server_start(SocketAddress *addr, Error **errp) } } -/* - * Hook into the BlockBackend notifiers to close the export when the - * backend is closed. - */ -typedef struct NBDCloseNotifier { - Notifier n; - NBDExport *exp; - QTAILQ_ENTRY(NBDCloseNotifier) next; -} NBDCloseNotifier; - -static QTAILQ_HEAD(, NBDCloseNotifier) close_notifiers = - QTAILQ_HEAD_INITIALIZER(close_notifiers); - -static void nbd_close_notifier(Notifier *n, void *data) -{ - NBDCloseNotifier *cn = DO_UPCAST(NBDCloseNotifier, n, n); - - notifier_remove(&cn->n); - QTAILQ_REMOVE(&close_notifiers, cn, next); - - nbd_export_close(cn->exp); - nbd_export_put(cn->exp); - g_free(cn); -} - void qmp_nbd_server_add(const char *device, bool has_writable, bool writable, Error **errp) { BlockBackend *blk; NBDExport *exp; - NBDCloseNotifier *n; if (server_fd == -1) { error_setg(errp, "NBD server not running"); @@ -113,19 +87,15 @@ void qmp_nbd_server_add(const char *device, bool has_writable, bool writable, nbd_export_set_name(exp, device); - n = g_new0(NBDCloseNotifier, 1); - n->n.notify = nbd_close_notifier; - n->exp = exp; - blk_add_close_notifier(blk, &n->n); - QTAILQ_INSERT_TAIL(&close_notifiers, n, next); + /* The list of named exports has a strong reference to this export now and + * our only way of accessing it is through nbd_export_find(), so we can drop + * the strong reference that is @exp. */ + nbd_export_put(exp); } void qmp_nbd_server_stop(Error **errp) { - while (!QTAILQ_EMPTY(&close_notifiers)) { - NBDCloseNotifier *cn = QTAILQ_FIRST(&close_notifiers); - nbd_close_notifier(&cn->n, nbd_export_get_blockdev(cn->exp)); - } + nbd_export_close_all(); if (server_fd != -1) { qemu_set_fd_handler(server_fd, NULL, NULL, NULL); diff --git a/nbd/server.c b/nbd/server.c index 7fa7f53..1ec79cf 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -64,6 +64,8 @@ struct NBDExport { QTAILQ_ENTRY(NBDExport) next; AioContext *ctx; + + Notifier eject_notifier; }; static QTAILQ_HEAD(, NBDExport) exports = QTAILQ_HEAD_INITIALIZER(exports); @@ -644,6 +646,12 @@ static void blk_aio_detach(void *opaque) exp->ctx = NULL; } +static void nbd_eject_notifier(Notifier *n, void *data) +{ + NBDExport *exp = container_of(n, NBDExport, eject_notifier); + nbd_export_close(exp); +} + NBDExport *nbd_export_new(BlockBackend *blk, off_t dev_offset, off_t size, uint32_t nbdflags, void (*close)(NBDExport *), Error **errp) @@ -666,6 +674,10 @@ NBDExport *nbd_export_new(BlockBackend *blk, off_t dev_offset, off_t size, exp->ctx = blk_get_aio_context(blk); blk_ref(blk); blk_add_aio_context_notifier(blk, blk_aio_attached, blk_aio_detach, exp); + + exp->eject_notifier.notify = nbd_eject_notifier; + blk_add_remove_bs_notifier(blk, &exp->eject_notifier); + /* * NBD exports are used for non-shared storage migration. Make sure * that BDRV_O_INACTIVE is cleared and the image is ready for write @@ -747,6 +759,7 @@ void nbd_export_put(NBDExport *exp) } if (exp->blk) { + notifier_remove(&exp->eject_notifier); blk_remove_aio_context_notifier(exp->blk, blk_aio_attached, blk_aio_detach, exp); blk_unref(exp->blk); -- cgit v1.1 From 033cb5659a1dce15643d2d5123615c26e24298ce Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 29 Jan 2016 16:36:07 +0100 Subject: block: Remove BDS close notifier It is unused now, so we can remove it. Signed-off-by: Max Reitz Reviewed-by: Fam Zheng Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block.c | 8 -------- block/block-backend.c | 7 ------- include/block/block.h | 1 - include/block/block_int.h | 2 -- include/sysemu/block-backend.h | 1 - 5 files changed, 19 deletions(-) diff --git a/block.c b/block.c index 41ab00e..f4312d9 100644 --- a/block.c +++ b/block.c @@ -259,7 +259,6 @@ BlockDriverState *bdrv_new(void) for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) { QLIST_INIT(&bs->op_blockers[i]); } - notifier_list_init(&bs->close_notifiers); notifier_with_return_list_init(&bs->before_write_notifiers); qemu_co_queue_init(&bs->throttled_reqs[0]); qemu_co_queue_init(&bs->throttled_reqs[1]); @@ -269,11 +268,6 @@ BlockDriverState *bdrv_new(void) return bs; } -void bdrv_add_close_notifier(BlockDriverState *bs, Notifier *notify) -{ - notifier_list_add(&bs->close_notifiers, notify); -} - BlockDriver *bdrv_find_format(const char *format_name) { BlockDriver *drv1; @@ -2157,8 +2151,6 @@ void bdrv_close(BlockDriverState *bs) bdrv_flush(bs); bdrv_drain(bs); /* in case flush left pending I/O */ - notifier_list_notify(&bs->close_notifiers, bs); - bdrv_release_named_dirty_bitmaps(bs); assert(QLIST_EMPTY(&bs->dirty_bitmaps)); diff --git a/block/block-backend.c b/block/block-backend.c index 1872191..621787c 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1146,13 +1146,6 @@ void blk_add_insert_bs_notifier(BlockBackend *blk, Notifier *notify) notifier_list_add(&blk->insert_bs_notifiers, notify); } -void blk_add_close_notifier(BlockBackend *blk, Notifier *notify) -{ - if (blk->bs) { - bdrv_add_close_notifier(blk->bs, notify); - } -} - void blk_io_plug(BlockBackend *blk) { if (blk->bs) { diff --git a/include/block/block.h b/include/block/block.h index 25f36dc..c7345de 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -226,7 +226,6 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, void bdrv_reopen_commit(BDRVReopenState *reopen_state); void bdrv_reopen_abort(BDRVReopenState *reopen_state); void bdrv_close(BlockDriverState *bs); -void bdrv_add_close_notifier(BlockDriverState *bs, Notifier *notify); int bdrv_read(BlockDriverState *bs, int64_t sector_num, uint8_t *buf, int nb_sectors); int bdrv_read_unthrottled(BlockDriverState *bs, int64_t sector_num, diff --git a/include/block/block_int.h b/include/block/block_int.h index ec31df1..8730cf6 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -403,8 +403,6 @@ struct BlockDriverState { BdrvChild *backing; BdrvChild *file; - NotifierList close_notifiers; - /* Callback before write request is processed */ NotifierWithReturnList before_write_notifiers; diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index e12be67..ae4efb4 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -166,7 +166,6 @@ void blk_remove_aio_context_notifier(BlockBackend *blk, void *opaque); void blk_add_remove_bs_notifier(BlockBackend *blk, Notifier *notify); void blk_add_insert_bs_notifier(BlockBackend *blk, Notifier *notify); -void blk_add_close_notifier(BlockBackend *blk, Notifier *notify); void blk_io_plug(BlockBackend *blk); void blk_io_unplug(BlockBackend *blk); BlockAcctStats *blk_get_stats(BlockBackend *blk); -- cgit v1.1 From 13855c6b9fa7f9d9e6d1f90377be0f678671073a Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 29 Jan 2016 16:36:08 +0100 Subject: block: Use blk_remove_bs() in blk_delete() Signed-off-by: Max Reitz Reviewed-by: Kevin Wolf Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- block/block-backend.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/block/block-backend.c b/block/block-backend.c index 621787c..7f5ad59 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -166,10 +166,7 @@ static void blk_delete(BlockBackend *blk) assert(!blk->refcnt); assert(!blk->dev); if (blk->bs) { - assert(blk->bs->blk == blk); - blk->bs->blk = NULL; - bdrv_unref(blk->bs); - blk->bs = NULL; + blk_remove_bs(blk); } assert(QLIST_EMPTY(&blk->remove_bs_notifiers.notifiers)); assert(QLIST_EMPTY(&blk->insert_bs_notifiers.notifiers)); @@ -351,6 +348,8 @@ void blk_hide_on_behalf_of_hmp_drive_del(BlockBackend *blk) */ void blk_remove_bs(BlockBackend *blk) { + assert(blk->bs->blk == blk); + notifier_list_notify(&blk->remove_bs_notifiers, blk); blk_update_root_state(blk); -- cgit v1.1 From 938abd4325951f94e87765b7daea61bf0bf71a5e Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 29 Jan 2016 16:36:09 +0100 Subject: blockdev: Use blk_remove_bs() in do_drive_del() Signed-off-by: Max Reitz Reviewed-by: Kevin Wolf Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- blockdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/blockdev.c b/blockdev.c index 1044a6a..09d4621 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2792,7 +2792,7 @@ void hmp_drive_del(Monitor *mon, const QDict *qdict) return; } - bdrv_close(bs); + blk_remove_bs(blk); } /* if we have a device attached to this BlockDriverState -- cgit v1.1 From 64dff52019367194699a772e3cc31941fd9752a8 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 29 Jan 2016 16:36:10 +0100 Subject: block: Make bdrv_close() static There are no users of bdrv_close() left, except for one of bdrv_open()'s failure paths, bdrv_close_all() and bdrv_delete(), and that is good. Make bdrv_close() static so nobody makes the mistake of directly using bdrv_close() again. Signed-off-by: Max Reitz Reviewed-by: Eric Blake Reviewed-by: Alberto Garcia Reviewed-by: Kevin Wolf Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- block.c | 4 +++- include/block/block.h | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/block.c b/block.c index f4312d9..e076f10 100644 --- a/block.c +++ b/block.c @@ -93,6 +93,8 @@ static void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs); /* If non-zero, use only whitelisted block drivers */ static int use_bdrv_whitelist; +static void bdrv_close(BlockDriverState *bs); + #ifdef _WIN32 static int is_windows_drive_prefix(const char *filename) { @@ -2134,7 +2136,7 @@ void bdrv_reopen_abort(BDRVReopenState *reopen_state) } -void bdrv_close(BlockDriverState *bs) +static void bdrv_close(BlockDriverState *bs) { BdrvAioNotifier *ban, *ban_next; diff --git a/include/block/block.h b/include/block/block.h index c7345de..4452b79 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -225,7 +225,6 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, Error **errp); void bdrv_reopen_commit(BDRVReopenState *reopen_state); void bdrv_reopen_abort(BDRVReopenState *reopen_state); -void bdrv_close(BlockDriverState *bs); int bdrv_read(BlockDriverState *bs, int64_t sector_num, uint8_t *buf, int nb_sectors); int bdrv_read_unthrottled(BlockDriverState *bs, int64_t sector_num, -- cgit v1.1 From 2c1d04e002dc91a6f34e8e683ea6b84f61a0b9fd Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 29 Jan 2016 16:36:11 +0100 Subject: block: Add list of all BlockDriverStates We need this list so that bdrv_close_all() can keep track of which BDSs are still open after having removed the BDSs from all of the BBs and having released all monitor BDS references. Signed-off-by: Max Reitz Reviewed-by: Kevin Wolf Reviewed-by: Fam Zheng Signed-off-by: Kevin Wolf --- block.c | 7 +++++++ include/block/block_int.h | 2 ++ 2 files changed, 9 insertions(+) diff --git a/block.c b/block.c index e076f10..d687d2c 100644 --- a/block.c +++ b/block.c @@ -79,6 +79,9 @@ struct BdrvStates bdrv_states = QTAILQ_HEAD_INITIALIZER(bdrv_states); static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states = QTAILQ_HEAD_INITIALIZER(graph_bdrv_states); +static QTAILQ_HEAD(, BlockDriverState) all_bdrv_states = + QTAILQ_HEAD_INITIALIZER(all_bdrv_states); + static QLIST_HEAD(, BlockDriver) bdrv_drivers = QLIST_HEAD_INITIALIZER(bdrv_drivers); @@ -267,6 +270,8 @@ BlockDriverState *bdrv_new(void) bs->refcnt = 1; bs->aio_context = qemu_get_aio_context(); + QTAILQ_INSERT_TAIL(&all_bdrv_states, bs, bs_list); + return bs; } @@ -2371,6 +2376,8 @@ static void bdrv_delete(BlockDriverState *bs) /* remove from list, if necessary */ bdrv_make_anon(bs); + QTAILQ_REMOVE(&all_bdrv_states, bs, bs_list); + g_free(bs); } diff --git a/include/block/block_int.h b/include/block/block_int.h index 8730cf6..1e4c518 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -443,6 +443,8 @@ struct BlockDriverState { QTAILQ_ENTRY(BlockDriverState) node_list; /* element of the list of "drives" the guest sees */ QTAILQ_ENTRY(BlockDriverState) device_list; + /* element of the list of all BlockDriverStates (all_bdrv_states) */ + QTAILQ_ENTRY(BlockDriverState) bs_list; QLIST_HEAD(, BdrvDirtyBitmap) dirty_bitmaps; int refcnt; -- cgit v1.1 From 9c4218e957331e1ba0ba7565730b0b71c49b8d70 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 29 Jan 2016 16:36:12 +0100 Subject: blockdev: Keep track of monitor-owned BDS As a side effect, we can now make x-blockdev-del's check whether a BDS is actually owned by the monitor explicit. Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- blockdev.c | 28 +++++++++++++++++++++++++++- include/block/block_int.h | 4 ++++ stubs/Makefile.objs | 1 + stubs/blockdev-close-all-bdrv-states.c | 5 +++++ 4 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 stubs/blockdev-close-all-bdrv-states.c diff --git a/blockdev.c b/blockdev.c index 09d4621..35e1e5c 100644 --- a/blockdev.c +++ b/blockdev.c @@ -50,6 +50,9 @@ #include "trace.h" #include "sysemu/arch_init.h" +static QTAILQ_HEAD(, BlockDriverState) monitor_bdrv_states = + QTAILQ_HEAD_INITIALIZER(monitor_bdrv_states); + static const char *const if_name[IF_COUNT] = { [IF_NONE] = "none", [IF_IDE] = "ide", @@ -702,6 +705,19 @@ fail: return NULL; } +void blockdev_close_all_bdrv_states(void) +{ + BlockDriverState *bs, *next_bs; + + QTAILQ_FOREACH_SAFE(bs, &monitor_bdrv_states, monitor_list, next_bs) { + AioContext *ctx = bdrv_get_aio_context(bs); + + aio_context_acquire(ctx); + bdrv_unref(bs); + aio_context_release(ctx); + } +} + static void qemu_opt_rename(QemuOpts *opts, const char *from, const char *to, Error **errp) { @@ -3875,12 +3891,15 @@ void qmp_blockdev_add(BlockdevOptions *options, Error **errp) if (!bs) { goto fail; } + + QTAILQ_INSERT_TAIL(&monitor_bdrv_states, bs, monitor_list); } if (bs && bdrv_key_required(bs)) { if (blk) { blk_unref(blk); } else { + QTAILQ_REMOVE(&monitor_bdrv_states, bs, monitor_list); bdrv_unref(bs); } error_setg(errp, "blockdev-add doesn't support encrypted devices"); @@ -3940,7 +3959,13 @@ void qmp_x_blockdev_del(bool has_id, const char *id, goto out; } - if (bs->refcnt > 1 || !QLIST_EMPTY(&bs->parents)) { + if (!blk && !bs->monitor_list.tqe_prev) { + error_setg(errp, "Node %s is not owned by the monitor", + bs->node_name); + goto out; + } + + if (bs->refcnt > 1) { error_setg(errp, "Block device %s is in use", bdrv_get_device_or_node_name(bs)); goto out; @@ -3950,6 +3975,7 @@ void qmp_x_blockdev_del(bool has_id, const char *id, if (blk) { blk_unref(blk); } else { + QTAILQ_REMOVE(&monitor_bdrv_states, bs, monitor_list); bdrv_unref(bs); } diff --git a/include/block/block_int.h b/include/block/block_int.h index 1e4c518..dd00d12 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -445,6 +445,8 @@ struct BlockDriverState { QTAILQ_ENTRY(BlockDriverState) device_list; /* element of the list of all BlockDriverStates (all_bdrv_states) */ QTAILQ_ENTRY(BlockDriverState) bs_list; + /* element of the list of monitor-owned BDS */ + QTAILQ_ENTRY(BlockDriverState) monitor_list; QLIST_HEAD(, BdrvDirtyBitmap) dirty_bitmaps; int refcnt; @@ -707,4 +709,6 @@ bool bdrv_requests_pending(BlockDriverState *bs); void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out); void bdrv_undo_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *in); +void blockdev_close_all_bdrv_states(void); + #endif /* BLOCK_INT_H */ diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs index d7898a0..e922de9 100644 --- a/stubs/Makefile.objs +++ b/stubs/Makefile.objs @@ -1,5 +1,6 @@ stub-obj-y += arch-query-cpu-def.o stub-obj-y += bdrv-commit-all.o +stub-obj-y += blockdev-close-all-bdrv-states.o stub-obj-y += clock-warp.o stub-obj-y += cpu-get-clock.o stub-obj-y += cpu-get-icount.o diff --git a/stubs/blockdev-close-all-bdrv-states.c b/stubs/blockdev-close-all-bdrv-states.c new file mode 100644 index 0000000..12d2442 --- /dev/null +++ b/stubs/blockdev-close-all-bdrv-states.c @@ -0,0 +1,5 @@ +#include "block/block_int.h" + +void blockdev_close_all_bdrv_states(void) +{ +} -- cgit v1.1 From d8da3cef3bc649492d190e029343293df1386027 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 29 Jan 2016 16:36:13 +0100 Subject: block: Add blk_remove_all_bs() When bdrv_close_all() is called, instead of force-closing all root BlockDriverStates, it is better to just drop the reference from all BlockBackends and let them be closed automatically. This prevents BDS from getting closed that are still referenced by other BDS, which may result in loss of cached data. This patch adds a function for doing that, but does not yet incorporate it in bdrv_close_all(). Signed-off-by: Max Reitz Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/block-backend.c | 15 +++++++++++++++ include/sysemu/block-backend.h | 1 + 2 files changed, 16 insertions(+) diff --git a/block/block-backend.c b/block/block-backend.c index 7f5ad59..ebdf78a 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -223,6 +223,21 @@ void blk_unref(BlockBackend *blk) } } +void blk_remove_all_bs(void) +{ + BlockBackend *blk; + + QTAILQ_FOREACH(blk, &blk_backends, link) { + AioContext *ctx = blk_get_aio_context(blk); + + aio_context_acquire(ctx); + if (blk->bs) { + blk_remove_bs(blk); + } + aio_context_release(ctx); + } +} + /* * Return the BlockBackend after @blk. * If @blk is null, return the first one. diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index ae4efb4..ec30331 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -68,6 +68,7 @@ BlockBackend *blk_new_open(const char *name, const char *filename, int blk_get_refcnt(BlockBackend *blk); void blk_ref(BlockBackend *blk); void blk_unref(BlockBackend *blk); +void blk_remove_all_bs(void); const char *blk_name(BlockBackend *blk); BlockBackend *blk_by_name(const char *name); BlockBackend *blk_next(BlockBackend *blk); -- cgit v1.1 From ca9bd24cf1d53775169ba9adc17e265554d1afed Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 29 Jan 2016 16:36:14 +0100 Subject: block: Rewrite bdrv_close_all() This patch rewrites bdrv_close_all(): Until now, all root BDSs have been force-closed. This is bad because it can lead to cached data not being flushed to disk. Instead, try to make all reference holders relinquish their reference voluntarily: 1. All BlockBackend users are handled by making all BBs simply eject their BDS tree. Since a BDS can never be on top of a BB, this will not cause any of the issues as seen with the force-closing of BDSs. The references will be relinquished and any further access to the BB will fail gracefully. 2. All BDSs which are owned by the monitor itself (because they do not have a BB) are relinquished next. 3. Besides BBs and the monitor, block jobs and other BDSs are the only things left that can hold a reference to BDSs. After every remaining block job has been canceled, there should not be any BDSs left (and the loop added here will always terminate (as long as NDEBUG is not defined), because either all_bdrv_states will be empty or there will not be any block job left to cancel, failing the assertion). Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block.c | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/block.c b/block.c index d687d2c..ff1aafc 100644 --- a/block.c +++ b/block.c @@ -2145,9 +2145,7 @@ static void bdrv_close(BlockDriverState *bs) { BdrvAioNotifier *ban, *ban_next; - if (bs->job) { - block_job_cancel_sync(bs->job); - } + assert(!bs->job); /* Disable I/O limits and drain all pending throttled requests */ if (bs->throttle_state) { @@ -2214,13 +2212,33 @@ static void bdrv_close(BlockDriverState *bs) void bdrv_close_all(void) { BlockDriverState *bs; + AioContext *aio_context; - QTAILQ_FOREACH(bs, &bdrv_states, device_list) { - AioContext *aio_context = bdrv_get_aio_context(bs); + /* Drop references from requests still in flight, such as canceled block + * jobs whose AIO context has not been polled yet */ + bdrv_drain_all(); - aio_context_acquire(aio_context); - bdrv_close(bs); - aio_context_release(aio_context); + blk_remove_all_bs(); + blockdev_close_all_bdrv_states(); + + /* Cancel all block jobs */ + while (!QTAILQ_EMPTY(&all_bdrv_states)) { + QTAILQ_FOREACH(bs, &all_bdrv_states, bs_list) { + aio_context = bdrv_get_aio_context(bs); + + aio_context_acquire(aio_context); + if (bs->job) { + block_job_cancel_sync(bs->job); + aio_context_release(aio_context); + break; + } + aio_context_release(aio_context); + } + + /* All the remaining BlockDriverStates are referenced directly or + * indirectly from block jobs, so there needs to be at least one BDS + * directly used by a block job */ + assert(bs); } } -- cgit v1.1 From 15a2b18fe5824388debe958e21c5dfa51457c7b6 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 29 Jan 2016 16:36:15 +0100 Subject: iotests: Add test for multiple BB on BDS tree This adds a test for having multiple BlockBackends in one BDS tree. In this case, there is one BB for the protocol BDS and one BB for the format BDS in a simple two-BDS tree (with the protocol BDS and BB added first). When bdrv_close_all() is executed, no cached data from any BDS should be lost; the protocol BDS may not be closed until the format BDS is closed. Otherwise, metadata updates may be lost. Signed-off-by: Max Reitz Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- tests/qemu-iotests/117 | 86 ++++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/117.out | 14 ++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 101 insertions(+) create mode 100755 tests/qemu-iotests/117 create mode 100644 tests/qemu-iotests/117.out diff --git a/tests/qemu-iotests/117 b/tests/qemu-iotests/117 new file mode 100755 index 0000000..969750d --- /dev/null +++ b/tests/qemu-iotests/117 @@ -0,0 +1,86 @@ +#!/bin/bash +# +# Test case for shared BDS between backend trees +# +# Copyright (C) 2016 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +# creator +owner=mreitz@redhat.com + +seq="$(basename $0)" +echo "QA output created by $seq" + +here="$PWD" +tmp=/tmp/$$ +status=1 # failure is the default! + +_cleanup() +{ + _cleanup_test_img +} +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter +. ./common.qemu + +_supported_fmt qcow2 +_supported_proto file +_supported_os Linux + +_make_test_img 64k + +_launch_qemu + +_send_qemu_cmd $QEMU_HANDLE \ + "{ 'execute': 'qmp_capabilities' }" \ + 'return' + +_send_qemu_cmd $QEMU_HANDLE \ + "{ 'execute': 'blockdev-add', + 'arguments': { 'options': { 'id': 'protocol', + 'driver': 'file', + 'filename': '$TEST_IMG' } } }" \ + 'return' + +_send_qemu_cmd $QEMU_HANDLE \ + "{ 'execute': 'blockdev-add', + 'arguments': { 'options': { 'id': 'format', + 'driver': '$IMGFMT', + 'file': 'protocol' } } }" \ + 'return' + +_send_qemu_cmd $QEMU_HANDLE \ + "{ 'execute': 'human-monitor-command', + 'arguments': { 'command-line': 'qemu-io format \"write -P 42 0 64k\"' } }" \ + 'return' + +_send_qemu_cmd $QEMU_HANDLE \ + "{ 'execute': 'quit' }" \ + 'return' + +wait=1 _cleanup_qemu + +_check_test_img + +$QEMU_IO -c 'read -P 42 0 64k' "$TEST_IMG" | _filter_qemu_io + +# success, all done +echo '*** done' +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/117.out b/tests/qemu-iotests/117.out new file mode 100644 index 0000000..f52dc1a --- /dev/null +++ b/tests/qemu-iotests/117.out @@ -0,0 +1,14 @@ +QA output created by 117 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=65536 +{"return": {}} +{"return": {}} +{"return": {}} +wrote 65536/65536 bytes at offset 0 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +{"return": ""} +{"return": {}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"} +No errors were found on the image. +read 65536/65536 bytes at offset 0 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +*** done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index ff1ff0d..e89f076 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -122,6 +122,7 @@ 114 rw auto quick 115 rw auto 116 rw auto quick +117 rw auto 118 rw auto 119 rw auto quick 120 rw auto quick -- cgit v1.1 From c78dc18295cb79c36d8993f245537f997cd9f2bb Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 29 Jan 2016 16:36:16 +0100 Subject: iotests: Add test for block jobs and BDS ejection Suggested-by: Paolo Bonzini Signed-off-by: Max Reitz Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- tests/qemu-iotests/141 | 186 +++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/141.out | 59 ++++++++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 246 insertions(+) create mode 100755 tests/qemu-iotests/141 create mode 100644 tests/qemu-iotests/141.out diff --git a/tests/qemu-iotests/141 b/tests/qemu-iotests/141 new file mode 100755 index 0000000..f7c28b4 --- /dev/null +++ b/tests/qemu-iotests/141 @@ -0,0 +1,186 @@ +#!/bin/bash +# +# Test case for ejecting BDSs with block jobs still running on them +# +# Copyright (C) 2016 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +# creator +owner=mreitz@redhat.com + +seq="$(basename $0)" +echo "QA output created by $seq" + +here="$PWD" +tmp=/tmp/$$ +status=1 # failure is the default! + +_cleanup() +{ + _cleanup_test_img + rm -f "$TEST_DIR/{b,m,o}.$IMGFMT" +} +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter +. ./common.qemu + +# Needs backing file and backing format support +_supported_fmt qcow2 qed +_supported_proto file +_supported_os Linux + + +test_blockjob() +{ + _send_qemu_cmd $QEMU_HANDLE \ + "{'execute': 'blockdev-add', + 'arguments': { + 'options': { + 'id': 'drv0', + 'driver': '$IMGFMT', + 'file': { + 'driver': 'file', + 'filename': '$TEST_IMG' + }}}}" \ + 'return' + + _send_qemu_cmd $QEMU_HANDLE \ + "$1" \ + "$2" \ + | _filter_img_create + + # We want this to return an error because the block job is still running + _send_qemu_cmd $QEMU_HANDLE \ + "{'execute': 'x-blockdev-remove-medium', + 'arguments': {'device': 'drv0'}}" \ + 'error' + + _send_qemu_cmd $QEMU_HANDLE \ + "{'execute': 'block-job-cancel', + 'arguments': {'device': 'drv0'}}" \ + "$3" + + _send_qemu_cmd $QEMU_HANDLE \ + "{'execute': 'x-blockdev-del', + 'arguments': {'id': 'drv0'}}" \ + 'return' +} + + +TEST_IMG="$TEST_DIR/b.$IMGFMT" _make_test_img 1M +TEST_IMG="$TEST_DIR/m.$IMGFMT" _make_test_img -b "$TEST_DIR/b.$IMGFMT" 1M +_make_test_img -b "$TEST_DIR/m.$IMGFMT" 1M + +_launch_qemu -nodefaults + +_send_qemu_cmd $QEMU_HANDLE \ + "{'execute': 'qmp_capabilities'}" \ + 'return' + +echo +echo '=== Testing drive-backup ===' +echo + +# drive-backup will not send BLOCK_JOB_READY by itself, and cancelling the job +# will consequently result in BLOCK_JOB_CANCELLED being emitted. + +test_blockjob \ + "{'execute': 'drive-backup', + 'arguments': {'device': 'drv0', + 'target': '$TEST_DIR/o.$IMGFMT', + 'format': '$IMGFMT', + 'sync': 'none'}}" \ + 'return' \ + 'BLOCK_JOB_CANCELLED' + +echo +echo '=== Testing drive-mirror ===' +echo + +# drive-mirror will send BLOCK_JOB_READY basically immediately, and cancelling +# the job will consequently result in BLOCK_JOB_COMPLETED being emitted. + +test_blockjob \ + "{'execute': 'drive-mirror', + 'arguments': {'device': 'drv0', + 'target': '$TEST_DIR/o.$IMGFMT', + 'format': '$IMGFMT', + 'sync': 'none'}}" \ + 'BLOCK_JOB_READY' \ + 'BLOCK_JOB_COMPLETED' + +echo +echo '=== Testing active block-commit ===' +echo + +# An active block-commit will send BLOCK_JOB_READY basically immediately, and +# cancelling the job will consequently result in BLOCK_JOB_COMPLETED being +# emitted. + +test_blockjob \ + "{'execute': 'block-commit', + 'arguments': {'device': 'drv0'}}" \ + 'BLOCK_JOB_READY' \ + 'BLOCK_JOB_COMPLETED' + +echo +echo '=== Testing non-active block-commit ===' +echo + +# Give block-commit something to work on, otherwise it would be done +# immediately, send a BLOCK_JOB_COMPLETED and ejecting the BDS would work just +# fine without the block job still running. + +$QEMU_IO -c 'write 0 1M' "$TEST_DIR/m.$IMGFMT" | _filter_qemu_io + +test_blockjob \ + "{'execute': 'block-commit', + 'arguments': {'device': 'drv0', + 'top': '$TEST_DIR/m.$IMGFMT', + 'speed': 1}}" \ + 'return' \ + 'BLOCK_JOB_CANCELLED' + +echo +echo '=== Testing block-stream ===' +echo + +# Give block-stream something to work on, otherwise it would be done +# immediately, send a BLOCK_JOB_COMPLETED and ejecting the BDS would work just +# fine without the block job still running. + +$QEMU_IO -c 'write 0 1M' "$TEST_DIR/b.$IMGFMT" | _filter_qemu_io + +# With some data to stream (and @speed set to 1), block-stream will not complete +# until we send the block-job-cancel command. Therefore, no event other than +# BLOCK_JOB_CANCELLED will be emitted. + +test_blockjob \ + "{'execute': 'block-stream', + 'arguments': {'device': 'drv0', + 'speed': 1}}" \ + 'return' \ + 'BLOCK_JOB_CANCELLED' + +_cleanup_qemu + +# success, all done +echo "*** done" +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/141.out b/tests/qemu-iotests/141.out new file mode 100644 index 0000000..adceac1 --- /dev/null +++ b/tests/qemu-iotests/141.out @@ -0,0 +1,59 @@ +QA output created by 141 +Formatting 'TEST_DIR/b.IMGFMT', fmt=IMGFMT size=1048576 +Formatting 'TEST_DIR/m.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/b.IMGFMT +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/m.IMGFMT +{"return": {}} + +=== Testing drive-backup === + +{"return": {}} +Formatting 'TEST_DIR/o.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.IMGFMT backing_fmt=IMGFMT +{"return": {}} +{"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device is in use by block job: backup"}} +{"return": {}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "drv0", "len": 1048576, "offset": 0, "speed": 0, "type": "backup"}} +{"return": {}} + +=== Testing drive-mirror === + +{"return": {}} +Formatting 'TEST_DIR/o.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.IMGFMT backing_fmt=IMGFMT +{"return": {}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "mirror"}} +{"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device is in use by block job: mirror"}} +{"return": {}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "mirror"}} +{"return": {}} + +=== Testing active block-commit === + +{"return": {}} +{"return": {}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}} +{"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device is in use by block job: commit"}} +{"return": {}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "drv0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}} +{"return": {}} + +=== Testing non-active block-commit === + +wrote 1048576/1048576 bytes at offset 0 +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +{"return": {}} +{"return": {}} +{"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device is in use by block job: commit"}} +{"return": {}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "drv0", "len": 1048576, "offset": 524288, "speed": 1, "type": "commit"}} +{"return": {}} + +=== Testing block-stream === + +wrote 1048576/1048576 bytes at offset 0 +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +{"return": {}} +{"return": {}} +{"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device is in use by block job: stream"}} +{"return": {}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "drv0", "len": 1048576, "offset": 524288, "speed": 1, "type": "stream"}} +{"return": {}} +*** done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index e89f076..cbc970a 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -143,5 +143,6 @@ 138 rw auto quick 139 rw auto quick 140 rw auto quick +141 rw auto quick 142 auto 143 auto quick -- cgit v1.1 From 1963f8d52e04a8f8b213e34e6a76fb286fb23ec1 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 23 Dec 2015 11:48:23 +0100 Subject: block: acquire in bdrv_query_image_info NFS calls aio_poll inside bdrv_get_allocated_size. This requires acquiring the AioContext. Signed-off-by: Paolo Bonzini Message-id: 1450867706-19860-1-git-send-email-pbonzini@redhat.com Reviewed-by: Fam Zheng Signed-off-by: Max Reitz --- block/qapi.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/block/qapi.c b/block/qapi.c index bbe0c9d..2e83105 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -211,11 +211,13 @@ void bdrv_query_image_info(BlockDriverState *bs, Error *err = NULL; ImageInfo *info; + aio_context_acquire(bdrv_get_aio_context(bs)); + size = bdrv_getlength(bs); if (size < 0) { error_setg_errno(errp, -size, "Can't get size of device '%s'", bdrv_get_device_name(bs)); - return; + goto out; } info = g_new0(ImageInfo, 1); @@ -283,10 +285,13 @@ void bdrv_query_image_info(BlockDriverState *bs, default: error_propagate(errp, err); qapi_free_ImageInfo(info); - return; + goto out; } *p_info = info; + +out: + aio_context_release(bdrv_get_aio_context(bs)); } /* @p_info will be set only on success. */ -- cgit v1.1 From 67a0fd2a9bca204d2b39f910a97c7137636a0715 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 26 Jan 2016 11:58:48 +0800 Subject: block: Add "file" output parameter to block status query functions The added parameter can be used to return the BDS pointer which the valid offset is referring to. Its value should be ignored unless BDRV_BLOCK_OFFSET_VALID in ret is set. Until block drivers fill in the right value, let's clear it explicitly right before calling .bdrv_get_block_status. The "bs->file" condition in bdrv_co_get_block_status is kept now to keep iotest case 102 passing, and will be fixed once all drivers return the right file pointer. Signed-off-by: Fam Zheng Message-id: 1453780743-16806-2-git-send-email-famz@redhat.com Reviewed-by: Max Reitz Signed-off-by: Max Reitz --- block/io.c | 40 ++++++++++++++++++++++++++++------------ block/iscsi.c | 6 ++++-- block/mirror.c | 3 ++- block/parallels.c | 2 +- block/qcow.c | 2 +- block/qcow2.c | 2 +- block/qed.c | 3 ++- block/raw-posix.c | 3 ++- block/raw_bsd.c | 3 ++- block/sheepdog.c | 2 +- block/vdi.c | 2 +- block/vmdk.c | 2 +- block/vpc.c | 2 +- block/vvfat.c | 2 +- include/block/block.h | 11 +++++++---- include/block/block_int.h | 3 ++- qemu-img.c | 13 +++++++++---- 17 files changed, 66 insertions(+), 35 deletions(-) diff --git a/block/io.c b/block/io.c index 5bb353a..ea040be 100644 --- a/block/io.c +++ b/block/io.c @@ -664,6 +664,7 @@ int bdrv_write_zeroes(BlockDriverState *bs, int64_t sector_num, int bdrv_make_zero(BlockDriverState *bs, BdrvRequestFlags flags) { int64_t target_sectors, ret, nb_sectors, sector_num = 0; + BlockDriverState *file; int n; target_sectors = bdrv_nb_sectors(bs); @@ -676,7 +677,7 @@ int bdrv_make_zero(BlockDriverState *bs, BdrvRequestFlags flags) if (nb_sectors <= 0) { return 0; } - ret = bdrv_get_block_status(bs, sector_num, nb_sectors, &n); + ret = bdrv_get_block_status(bs, sector_num, nb_sectors, &n, &file); if (ret < 0) { error_report("error getting block status at sector %" PRId64 ": %s", sector_num, strerror(-ret)); @@ -1466,6 +1467,7 @@ int bdrv_flush_all(void) typedef struct BdrvCoGetBlockStatusData { BlockDriverState *bs; BlockDriverState *base; + BlockDriverState **file; int64_t sector_num; int nb_sectors; int *pnum; @@ -1487,10 +1489,14 @@ typedef struct BdrvCoGetBlockStatusData { * * 'nb_sectors' is the max value 'pnum' should be set to. If nb_sectors goes * beyond the end of the disk image it will be clamped. + * + * If returned value is positive and BDRV_BLOCK_OFFSET_VALID bit is set, 'file' + * points to the BDS which the sector range is allocated in. */ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs, int64_t sector_num, - int nb_sectors, int *pnum) + int nb_sectors, int *pnum, + BlockDriverState **file) { int64_t total_sectors; int64_t n; @@ -1520,7 +1526,9 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs, return ret; } - ret = bs->drv->bdrv_co_get_block_status(bs, sector_num, nb_sectors, pnum); + *file = NULL; + ret = bs->drv->bdrv_co_get_block_status(bs, sector_num, nb_sectors, pnum, + file); if (ret < 0) { *pnum = 0; return ret; @@ -1529,7 +1537,7 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs, if (ret & BDRV_BLOCK_RAW) { assert(ret & BDRV_BLOCK_OFFSET_VALID); return bdrv_get_block_status(bs->file->bs, ret >> BDRV_SECTOR_BITS, - *pnum, pnum); + *pnum, pnum, file); } if (ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ZERO)) { @@ -1549,10 +1557,11 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs, if (bs->file && (ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) && (ret & BDRV_BLOCK_OFFSET_VALID)) { + BlockDriverState *file2; int file_pnum; ret2 = bdrv_co_get_block_status(bs->file->bs, ret >> BDRV_SECTOR_BITS, - *pnum, &file_pnum); + *pnum, &file_pnum, &file2); if (ret2 >= 0) { /* Ignore errors. This is just providing extra information, it * is useful but not necessary. @@ -1577,14 +1586,15 @@ static int64_t coroutine_fn bdrv_co_get_block_status_above(BlockDriverState *bs, BlockDriverState *base, int64_t sector_num, int nb_sectors, - int *pnum) + int *pnum, + BlockDriverState **file) { BlockDriverState *p; int64_t ret = 0; assert(bs != base); for (p = bs; p != base; p = backing_bs(p)) { - ret = bdrv_co_get_block_status(p, sector_num, nb_sectors, pnum); + ret = bdrv_co_get_block_status(p, sector_num, nb_sectors, pnum, file); if (ret < 0 || ret & BDRV_BLOCK_ALLOCATED) { break; } @@ -1603,7 +1613,8 @@ static void coroutine_fn bdrv_get_block_status_above_co_entry(void *opaque) data->ret = bdrv_co_get_block_status_above(data->bs, data->base, data->sector_num, data->nb_sectors, - data->pnum); + data->pnum, + data->file); data->done = true; } @@ -1615,12 +1626,14 @@ static void coroutine_fn bdrv_get_block_status_above_co_entry(void *opaque) int64_t bdrv_get_block_status_above(BlockDriverState *bs, BlockDriverState *base, int64_t sector_num, - int nb_sectors, int *pnum) + int nb_sectors, int *pnum, + BlockDriverState **file) { Coroutine *co; BdrvCoGetBlockStatusData data = { .bs = bs, .base = base, + .file = file, .sector_num = sector_num, .nb_sectors = nb_sectors, .pnum = pnum, @@ -1644,16 +1657,19 @@ int64_t bdrv_get_block_status_above(BlockDriverState *bs, int64_t bdrv_get_block_status(BlockDriverState *bs, int64_t sector_num, - int nb_sectors, int *pnum) + int nb_sectors, int *pnum, + BlockDriverState **file) { return bdrv_get_block_status_above(bs, backing_bs(bs), - sector_num, nb_sectors, pnum); + sector_num, nb_sectors, pnum, file); } int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum) { - int64_t ret = bdrv_get_block_status(bs, sector_num, nb_sectors, pnum); + BlockDriverState *file; + int64_t ret = bdrv_get_block_status(bs, sector_num, nb_sectors, pnum, + &file); if (ret < 0) { return ret; } diff --git a/block/iscsi.c b/block/iscsi.c index bffd707..e182557 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -532,7 +532,8 @@ static bool iscsi_allocationmap_is_allocated(IscsiLun *iscsilun, static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs, int64_t sector_num, - int nb_sectors, int *pnum) + int nb_sectors, int *pnum, + BlockDriverState **file) { IscsiLun *iscsilun = bs->opaque; struct scsi_get_lba_status *lbas = NULL; @@ -650,7 +651,8 @@ static int coroutine_fn iscsi_co_readv(BlockDriverState *bs, !iscsi_allocationmap_is_allocated(iscsilun, sector_num, nb_sectors)) { int64_t ret; int pnum; - ret = iscsi_co_get_block_status(bs, sector_num, INT_MAX, &pnum); + BlockDriverState *file; + ret = iscsi_co_get_block_status(bs, sector_num, INT_MAX, &pnum, &file); if (ret < 0) { return ret; } diff --git a/block/mirror.c b/block/mirror.c index e9e151c..2c0edfa 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -168,6 +168,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s) MirrorOp *op; int pnum; int64_t ret; + BlockDriverState *file; max_iov = MIN(source->bl.max_iov, s->target->bl.max_iov); @@ -306,7 +307,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s) trace_mirror_one_iteration(s, sector_num, nb_sectors); ret = bdrv_get_block_status_above(source, NULL, sector_num, - nb_sectors, &pnum); + nb_sectors, &pnum, &file); if (ret < 0 || pnum < nb_sectors || (ret & BDRV_BLOCK_DATA && !(ret & BDRV_BLOCK_ZERO))) { bdrv_aio_readv(source, sector_num, &op->qiov, nb_sectors, diff --git a/block/parallels.c b/block/parallels.c index ee39081..e2de308 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -261,7 +261,7 @@ static coroutine_fn int parallels_co_flush_to_os(BlockDriverState *bs) static int64_t coroutine_fn parallels_co_get_block_status(BlockDriverState *bs, - int64_t sector_num, int nb_sectors, int *pnum) + int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **file) { BDRVParallelsState *s = bs->opaque; int64_t offset; diff --git a/block/qcow.c b/block/qcow.c index afed18f..4202797 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -489,7 +489,7 @@ static uint64_t get_cluster_offset(BlockDriverState *bs, } static int64_t coroutine_fn qcow_co_get_block_status(BlockDriverState *bs, - int64_t sector_num, int nb_sectors, int *pnum) + int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **file) { BDRVQcowState *s = bs->opaque; int index_in_cluster, n; diff --git a/block/qcow2.c b/block/qcow2.c index fd8436c..d4ea6b4 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1330,7 +1330,7 @@ static void qcow2_join_options(QDict *options, QDict *old_options) } static int64_t coroutine_fn qcow2_co_get_block_status(BlockDriverState *bs, - int64_t sector_num, int nb_sectors, int *pnum) + int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **file) { BDRVQcow2State *s = bs->opaque; uint64_t cluster_offset; diff --git a/block/qed.c b/block/qed.c index 0c870cd..8f6f841 100644 --- a/block/qed.c +++ b/block/qed.c @@ -725,7 +725,8 @@ static void qed_is_allocated_cb(void *opaque, int ret, uint64_t offset, size_t l static int64_t coroutine_fn bdrv_qed_co_get_block_status(BlockDriverState *bs, int64_t sector_num, - int nb_sectors, int *pnum) + int nb_sectors, int *pnum, + BlockDriverState **file) { BDRVQEDState *s = bs->opaque; size_t len = (size_t)nb_sectors * BDRV_SECTOR_SIZE; diff --git a/block/raw-posix.c b/block/raw-posix.c index 6df3067..3ef9b25 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -1818,7 +1818,8 @@ static int find_allocation(BlockDriverState *bs, off_t start, */ static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs, int64_t sector_num, - int nb_sectors, int *pnum) + int nb_sectors, int *pnum, + BlockDriverState **file) { off_t start, data = 0, hole = 0; int64_t total_size; diff --git a/block/raw_bsd.c b/block/raw_bsd.c index bcaee11..9a8933b 100644 --- a/block/raw_bsd.c +++ b/block/raw_bsd.c @@ -115,7 +115,8 @@ fail: static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs, int64_t sector_num, - int nb_sectors, int *pnum) + int nb_sectors, int *pnum, + BlockDriverState **file) { *pnum = nb_sectors; return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_DATA | diff --git a/block/sheepdog.c b/block/sheepdog.c index ff89298..2ea05a6 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -2708,7 +2708,7 @@ retry: static coroutine_fn int64_t sd_co_get_block_status(BlockDriverState *bs, int64_t sector_num, int nb_sectors, - int *pnum) + int *pnum, BlockDriverState **file) { BDRVSheepdogState *s = bs->opaque; SheepdogInode *inode = &s->inode; diff --git a/block/vdi.c b/block/vdi.c index 61bcd54..294c438 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -527,7 +527,7 @@ static int vdi_reopen_prepare(BDRVReopenState *state, } static int64_t coroutine_fn vdi_co_get_block_status(BlockDriverState *bs, - int64_t sector_num, int nb_sectors, int *pnum) + int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **file) { /* TODO: Check for too large sector_num (in bdrv_is_allocated or here). */ BDRVVdiState *s = (BDRVVdiState *)bs->opaque; diff --git a/block/vmdk.c b/block/vmdk.c index 4a5850b..109fd5f 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1261,7 +1261,7 @@ static inline uint64_t vmdk_find_index_in_cluster(VmdkExtent *extent, } static int64_t coroutine_fn vmdk_co_get_block_status(BlockDriverState *bs, - int64_t sector_num, int nb_sectors, int *pnum) + int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **file) { BDRVVmdkState *s = bs->opaque; int64_t index_in_cluster, n, ret; diff --git a/block/vpc.c b/block/vpc.c index d852f96..a070307 100644 --- a/block/vpc.c +++ b/block/vpc.c @@ -579,7 +579,7 @@ static coroutine_fn int vpc_co_write(BlockDriverState *bs, int64_t sector_num, } static int64_t coroutine_fn vpc_co_get_block_status(BlockDriverState *bs, - int64_t sector_num, int nb_sectors, int *pnum) + int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **file) { BDRVVPCState *s = bs->opaque; VHDFooter *footer = (VHDFooter*) s->footer_buf; diff --git a/block/vvfat.c b/block/vvfat.c index 2ea5a4a..b8d29e1 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -2884,7 +2884,7 @@ static coroutine_fn int vvfat_co_write(BlockDriverState *bs, int64_t sector_num, } static int64_t coroutine_fn vvfat_co_get_block_status(BlockDriverState *bs, - int64_t sector_num, int nb_sectors, int* n) + int64_t sector_num, int nb_sectors, int *n, BlockDriverState **file) { BDRVVVFATState* s = bs->opaque; *n = s->sector_count - sector_num; diff --git a/include/block/block.h b/include/block/block.h index 4452b79..0035ad8 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -111,9 +111,10 @@ typedef struct HDGeometry { /* * Allocation status flags - * BDRV_BLOCK_DATA: data is read from bs->file or another file + * BDRV_BLOCK_DATA: data is read from a file returned by bdrv_get_block_status. * BDRV_BLOCK_ZERO: sectors read as zero - * BDRV_BLOCK_OFFSET_VALID: sector stored in bs->file as raw data + * BDRV_BLOCK_OFFSET_VALID: sector stored as raw data in a file returned by + * bdrv_get_block_status. * BDRV_BLOCK_ALLOCATED: the content of the block is determined by this * layer (as opposed to the backing file) * BDRV_BLOCK_RAW: used internally to indicate that the request @@ -384,11 +385,13 @@ int bdrv_has_zero_init(BlockDriverState *bs); bool bdrv_unallocated_blocks_are_zero(BlockDriverState *bs); bool bdrv_can_write_zeroes_with_unmap(BlockDriverState *bs); int64_t bdrv_get_block_status(BlockDriverState *bs, int64_t sector_num, - int nb_sectors, int *pnum); + int nb_sectors, int *pnum, + BlockDriverState **file); int64_t bdrv_get_block_status_above(BlockDriverState *bs, BlockDriverState *base, int64_t sector_num, - int nb_sectors, int *pnum); + int nb_sectors, int *pnum, + BlockDriverState **file); int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum); int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base, diff --git a/include/block/block_int.h b/include/block/block_int.h index dd00d12..9ef823a 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -166,7 +166,8 @@ struct BlockDriver { int coroutine_fn (*bdrv_co_discard)(BlockDriverState *bs, int64_t sector_num, int nb_sectors); int64_t coroutine_fn (*bdrv_co_get_block_status)(BlockDriverState *bs, - int64_t sector_num, int nb_sectors, int *pnum); + int64_t sector_num, int nb_sectors, int *pnum, + BlockDriverState **file); /* * Invalidate any cached meta-data. diff --git a/qemu-img.c b/qemu-img.c index 33e451c..e653b2f 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1072,13 +1072,15 @@ static int img_compare(int argc, char **argv) for (;;) { int64_t status1, status2; + BlockDriverState *file; + nb_sectors = sectors_to_process(total_sectors, sector_num); if (nb_sectors <= 0) { break; } status1 = bdrv_get_block_status_above(bs1, NULL, sector_num, total_sectors1 - sector_num, - &pnum1); + &pnum1, &file); if (status1 < 0) { ret = 3; error_report("Sector allocation test failed for %s", filename1); @@ -1088,7 +1090,7 @@ static int img_compare(int argc, char **argv) status2 = bdrv_get_block_status_above(bs2, NULL, sector_num, total_sectors2 - sector_num, - &pnum2); + &pnum2, &file); if (status2 < 0) { ret = 3; error_report("Sector allocation test failed for %s", filename2); @@ -1271,9 +1273,10 @@ static int convert_iteration_sectors(ImgConvertState *s, int64_t sector_num) n = MIN(s->total_sectors - sector_num, BDRV_REQUEST_MAX_SECTORS); if (s->sector_next_status <= sector_num) { + BlockDriverState *file; ret = bdrv_get_block_status(blk_bs(s->src[s->src_cur]), sector_num - s->src_cur_offset, - n, &n); + n, &n, &file); if (ret < 0) { return ret; } @@ -2201,6 +2204,7 @@ static int get_block_status(BlockDriverState *bs, int64_t sector_num, { int64_t ret; int depth; + BlockDriverState *file; /* As an optimization, we could cache the current range of unallocated * clusters in each file of the chain, and avoid querying the same @@ -2209,7 +2213,8 @@ static int get_block_status(BlockDriverState *bs, int64_t sector_num, depth = 0; for (;;) { - ret = bdrv_get_block_status(bs, sector_num, nb_sectors, &nb_sectors); + ret = bdrv_get_block_status(bs, sector_num, nb_sectors, &nb_sectors, + &file); if (ret < 0) { return ret; } -- cgit v1.1 From 3064bf6fffe4705d983b8cecfbbe3de3e5a75312 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 26 Jan 2016 11:58:49 +0800 Subject: qcow: Assign bs->file->bs to file in qcow_co_get_block_status Reviewed-by: Eric Blake Reviewed-by: Stefan Hajnoczi Signed-off-by: Fam Zheng Message-id: 1453780743-16806-3-git-send-email-famz@redhat.com Signed-off-by: Max Reitz --- block/qcow.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/qcow.c b/block/qcow.c index 4202797..251910c 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -510,6 +510,7 @@ static int64_t coroutine_fn qcow_co_get_block_status(BlockDriverState *bs, return BDRV_BLOCK_DATA; } cluster_offset |= (index_in_cluster << BDRV_SECTOR_BITS); + *file = bs->file->bs; return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | cluster_offset; } -- cgit v1.1 From 178b4db7e5dc6c5bbaa24a72fdb232ec259c26eb Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 26 Jan 2016 11:58:50 +0800 Subject: qcow2: Assign bs->file->bs to file in qcow2_co_get_block_status Reviewed-by: Stefan Hajnoczi Signed-off-by: Fam Zheng Message-id: 1453780743-16806-4-git-send-email-famz@redhat.com Signed-off-by: Max Reitz --- block/qcow2.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/qcow2.c b/block/qcow2.c index d4ea6b4..8babecd 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1349,6 +1349,7 @@ static int64_t coroutine_fn qcow2_co_get_block_status(BlockDriverState *bs, !s->cipher) { index_in_cluster = sector_num & (s->cluster_sectors - 1); cluster_offset |= (index_in_cluster << BDRV_SECTOR_BITS); + *file = bs->file->bs; status |= BDRV_BLOCK_OFFSET_VALID | cluster_offset; } if (ret == QCOW2_CLUSTER_ZERO) { -- cgit v1.1 From 02650acbc622b51b3428630b2f60ca1966caa1e8 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 26 Jan 2016 11:58:51 +0800 Subject: raw: Assign bs to file in raw_co_get_block_status Signed-off-by: Fam Zheng Message-id: 1453780743-16806-5-git-send-email-famz@redhat.com Reviewed-by: Max Reitz Signed-off-by: Max Reitz --- block/raw-posix.c | 1 + block/raw_bsd.c | 1 + 2 files changed, 2 insertions(+) diff --git a/block/raw-posix.c b/block/raw-posix.c index 3ef9b25..8866121 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -1861,6 +1861,7 @@ static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs, *pnum = MIN(nb_sectors, (data - start) / BDRV_SECTOR_SIZE); ret = BDRV_BLOCK_ZERO; } + *file = bs; return ret | BDRV_BLOCK_OFFSET_VALID | start; } diff --git a/block/raw_bsd.c b/block/raw_bsd.c index 9a8933b..ea16a23 100644 --- a/block/raw_bsd.c +++ b/block/raw_bsd.c @@ -119,6 +119,7 @@ static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs, BlockDriverState **file) { *pnum = nb_sectors; + *file = bs->file->bs; return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_DATA | (sector_num << BDRV_SECTOR_BITS); } -- cgit v1.1 From 3399833f1412e72e6f9e6997775dfdf12d624eaf Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 26 Jan 2016 11:58:52 +0800 Subject: iscsi: Assign bs to file in iscsi_co_get_block_status Reviewed-by: Eric Blake Reviewed-by: Stefan Hajnoczi Signed-off-by: Fam Zheng Message-id: 1453780743-16806-6-git-send-email-famz@redhat.com Signed-off-by: Max Reitz --- block/iscsi.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/block/iscsi.c b/block/iscsi.c index e182557..9fe76f4 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -625,6 +625,9 @@ out: if (iTask.task != NULL) { scsi_free_scsi_task(iTask.task); } + if (ret > 0 && ret & BDRV_BLOCK_OFFSET_VALID) { + *file = bs; + } return ret; } -- cgit v1.1 From ddf4987d76ebc356da96f6901c1af970ef421da6 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 26 Jan 2016 11:58:53 +0800 Subject: parallels: Assign bs->file->bs to file in parallels_co_get_block_status Reviewed-by: Eric Blake Reviewed-by: Stefan Hajnoczi Signed-off-by: Fam Zheng Message-id: 1453780743-16806-7-git-send-email-famz@redhat.com Signed-off-by: Max Reitz --- block/parallels.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/parallels.c b/block/parallels.c index e2de308..645521d 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -274,6 +274,7 @@ static int64_t coroutine_fn parallels_co_get_block_status(BlockDriverState *bs, return 0; } + *file = bs->file->bs; return (offset << BDRV_SECTOR_BITS) | BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID; } -- cgit v1.1 From 53f1dfd1ff5a603ea1bd67b925758d22f54e1f8a Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 26 Jan 2016 11:58:54 +0800 Subject: qed: Assign bs->file->bs to file in bdrv_qed_co_get_block_status Reviewed-by: Eric Blake Reviewed-by: Stefan Hajnoczi Signed-off-by: Fam Zheng Message-id: 1453780743-16806-8-git-send-email-famz@redhat.com Signed-off-by: Max Reitz --- block/qed.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/block/qed.c b/block/qed.c index 8f6f841..404be1e 100644 --- a/block/qed.c +++ b/block/qed.c @@ -693,6 +693,7 @@ typedef struct { uint64_t pos; int64_t status; int *pnum; + BlockDriverState **file; } QEDIsAllocatedCB; static void qed_is_allocated_cb(void *opaque, int ret, uint64_t offset, size_t len) @@ -704,6 +705,7 @@ static void qed_is_allocated_cb(void *opaque, int ret, uint64_t offset, size_t l case QED_CLUSTER_FOUND: offset |= qed_offset_into_cluster(s, cb->pos); cb->status = BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | offset; + *cb->file = cb->bs->file->bs; break; case QED_CLUSTER_ZERO: cb->status = BDRV_BLOCK_ZERO; @@ -735,6 +737,7 @@ static int64_t coroutine_fn bdrv_qed_co_get_block_status(BlockDriverState *bs, .pos = (uint64_t)sector_num * BDRV_SECTOR_SIZE, .status = BDRV_BLOCK_OFFSET_MASK, .pnum = pnum, + .file = file, }; QEDRequest request = { .l2_table = NULL }; -- cgit v1.1 From d234c929310a322357ed4323c7014605449f5802 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 26 Jan 2016 11:58:55 +0800 Subject: sheepdog: Assign bs to file in sd_co_get_block_status Reviewed-by: Stefan Hajnoczi Signed-off-by: Fam Zheng Message-id: 1453780743-16806-9-git-send-email-famz@redhat.com Signed-off-by: Max Reitz --- block/sheepdog.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/block/sheepdog.c b/block/sheepdog.c index 2ea05a6..a0098c1 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -2739,6 +2739,9 @@ sd_co_get_block_status(BlockDriverState *bs, int64_t sector_num, int nb_sectors, if (*pnum > nb_sectors) { *pnum = nb_sectors; } + if (ret > 0 && ret & BDRV_BLOCK_OFFSET_VALID) { + *file = bs; + } return ret; } -- cgit v1.1 From 8bfb137152d2195c70e99ed3269a3f4825ffc614 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 26 Jan 2016 11:58:56 +0800 Subject: vdi: Assign bs->file->bs to file in vdi_co_get_block_status Reviewed-by: Stefan Hajnoczi Signed-off-by: Fam Zheng Message-id: 1453780743-16806-10-git-send-email-famz@redhat.com Signed-off-by: Max Reitz --- block/vdi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/vdi.c b/block/vdi.c index 294c438..b403243 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -551,6 +551,7 @@ static int64_t coroutine_fn vdi_co_get_block_status(BlockDriverState *bs, offset = s->header.offset_data + (uint64_t)bmap_entry * s->block_size + sector_in_block * SECTOR_SIZE; + *file = bs->file->bs; return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | offset; } -- cgit v1.1 From 7429e207883fb6f6bd46cefee4c79771e50f35f4 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 26 Jan 2016 11:58:57 +0800 Subject: vpc: Assign bs->file->bs to file in vpc_co_get_block_status Reviewed-by: Stefan Hajnoczi Signed-off-by: Fam Zheng Message-id: 1453780743-16806-11-git-send-email-famz@redhat.com Signed-off-by: Max Reitz --- block/vpc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/block/vpc.c b/block/vpc.c index a070307..f504536 100644 --- a/block/vpc.c +++ b/block/vpc.c @@ -589,6 +589,7 @@ static int64_t coroutine_fn vpc_co_get_block_status(BlockDriverState *bs, if (be32_to_cpu(footer->type) == VHD_FIXED) { *pnum = nb_sectors; + *file = bs->file->bs; return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_DATA | (sector_num << BDRV_SECTOR_BITS); } @@ -610,6 +611,7 @@ static int64_t coroutine_fn vpc_co_get_block_status(BlockDriverState *bs, /* *pnum can't be greater than one block for allocated * sectors since there is always a bitmap in between. */ if (allocated) { + *file = bs->file->bs; return BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | start; } if (nb_sectors == 0) { -- cgit v1.1 From d0a18f10251f515c86dcaec5bdf979a4e07fafc5 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 26 Jan 2016 11:58:58 +0800 Subject: vmdk: Fix calculation of block status's offset "offset" is the offset of cluster and sector_num doesn't necessarily refer to the start of it, it should add index_in_cluster. Signed-off-by: Fam Zheng Message-id: 1453780743-16806-12-git-send-email-famz@redhat.com Reviewed-by: Max Reitz Signed-off-by: Max Reitz --- block/vmdk.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 109fd5f..9d5a18a 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1278,6 +1278,7 @@ static int64_t coroutine_fn vmdk_co_get_block_status(BlockDriverState *bs, 0, 0); qemu_co_mutex_unlock(&s->lock); + index_in_cluster = vmdk_find_index_in_cluster(extent, sector_num); switch (ret) { case VMDK_ERROR: ret = -EIO; @@ -1291,13 +1292,14 @@ static int64_t coroutine_fn vmdk_co_get_block_status(BlockDriverState *bs, case VMDK_OK: ret = BDRV_BLOCK_DATA; if (extent->file == bs->file && !extent->compressed) { - ret |= BDRV_BLOCK_OFFSET_VALID | offset; + ret |= BDRV_BLOCK_OFFSET_VALID; + ret |= (offset + (index_in_cluster << BDRV_SECTOR_BITS)) + & BDRV_BLOCK_OFFSET_MASK; } break; } - index_in_cluster = vmdk_find_index_in_cluster(extent, sector_num); n = extent->cluster_sectors - index_in_cluster; if (n > nb_sectors) { n = nb_sectors; -- cgit v1.1 From e0f100f57ceee919a7df7af316961b175ffef4e6 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 26 Jan 2016 11:58:59 +0800 Subject: vmdk: Return extent's file in bdrv_get_block_status Signed-off-by: Fam Zheng Message-id: 1453780743-16806-13-git-send-email-famz@redhat.com Reviewed-by: Max Reitz Signed-off-by: Max Reitz --- block/vmdk.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 9d5a18a..a8db5d9 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1291,12 +1291,12 @@ static int64_t coroutine_fn vmdk_co_get_block_status(BlockDriverState *bs, break; case VMDK_OK: ret = BDRV_BLOCK_DATA; - if (extent->file == bs->file && !extent->compressed) { + if (!extent->compressed) { ret |= BDRV_BLOCK_OFFSET_VALID; ret |= (offset + (index_in_cluster << BDRV_SECTOR_BITS)) & BDRV_BLOCK_OFFSET_MASK; } - + *file = extent->file->bs; break; } -- cgit v1.1 From ac987b30d0a97f14b82584921fc5ab3cd88c431b Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 26 Jan 2016 11:59:00 +0800 Subject: block: Use returned *file in bdrv_co_get_block_status Now that all drivers return the right "file" pointer, we can use it. Signed-off-by: Fam Zheng Reviewed-by: Max Reitz Message-id: 1453780743-16806-14-git-send-email-famz@redhat.com Signed-off-by: Max Reitz --- block/io.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/io.c b/block/io.c index ea040be..343ff1f 100644 --- a/block/io.c +++ b/block/io.c @@ -1554,13 +1554,13 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs, } } - if (bs->file && + if (*file && *file != bs && (ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) && (ret & BDRV_BLOCK_OFFSET_VALID)) { BlockDriverState *file2; int file_pnum; - ret2 = bdrv_co_get_block_status(bs->file->bs, ret >> BDRV_SECTOR_BITS, + ret2 = bdrv_co_get_block_status(*file, ret >> BDRV_SECTOR_BITS, *pnum, &file_pnum, &file2); if (ret2 >= 0) { /* Ignore errors. This is just providing extra information, it -- cgit v1.1 From 9e4303400801e62e8e357decdb487834582459e8 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 26 Jan 2016 11:59:01 +0800 Subject: qemu-img: In "map", use the returned "file" from bdrv_get_block_status Now all drivers should return a correct "file", we can make use of it, even with the recursion into backing chain above. Reviewed-by: Eric Blake Reviewed-by: Stefan Hajnoczi Signed-off-by: Fam Zheng Message-id: 1453780743-16806-15-git-send-email-famz@redhat.com Signed-off-by: Max Reitz --- qemu-img.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qemu-img.c b/qemu-img.c index e653b2f..c8bc63f 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -2236,7 +2236,7 @@ static int get_block_status(BlockDriverState *bs, int64_t sector_num, e->flags = ret & ~BDRV_BLOCK_OFFSET_MASK; e->offset = ret & BDRV_BLOCK_OFFSET_MASK; e->depth = depth; - e->bs = bs; + e->bs = file; return 0; } -- cgit v1.1 From 16b0d555861b732c1dd76a986697214de1d774d5 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 26 Jan 2016 11:59:02 +0800 Subject: qemu-img: Make MapEntry a QAPI struct The "flags" bit mask is expanded to two booleans, "data" and "zero"; "bs" is replaced with "filename" string. Refactor the merge conditions in img_map() into entry_mergeable(). Reviewed-by: Eric Blake Reviewed-by: Stefan Hajnoczi Signed-off-by: Fam Zheng Message-id: 1453780743-16806-16-git-send-email-famz@redhat.com Signed-off-by: Max Reitz --- qapi/block-core.json | 27 ++++++++++++++++++++ qemu-img.c | 71 +++++++++++++++++++++++++++++++--------------------- 2 files changed, 69 insertions(+), 29 deletions(-) diff --git a/qapi/block-core.json b/qapi/block-core.json index 628a41d..33012b8 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -186,6 +186,33 @@ '*fragmented-clusters': 'int', '*compressed-clusters': 'int' } } ## +# @MapEntry: +# +# Mapping information from a virtual block range to a host file range +# +# @start: the start byte of the mapped virtual range +# +# @length: the number of bytes of the mapped virtual range +# +# @data: whether the mapped range has data +# +# @zero: whether the virtual blocks are zeroed +# +# @depth: the depth of the mapping +# +# @offset: #optional the offset in file that the virtual sectors are mapped to +# +# @filename: #optional filename that is referred to by @offset +# +# Since: 2.6 +# +## +{ 'struct': 'MapEntry', + 'data': {'start': 'int', 'length': 'int', 'data': 'bool', + 'zero': 'bool', 'depth': 'int', '*offset': 'int', + '*filename': 'str' } } + +## # @BlockdevCacheInfo # # Cache mode information for a block device diff --git a/qemu-img.c b/qemu-img.c index c8bc63f..f121980 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -2147,47 +2147,37 @@ static int img_info(int argc, char **argv) return 0; } - -typedef struct MapEntry { - int flags; - int depth; - int64_t start; - int64_t length; - int64_t offset; - BlockDriverState *bs; -} MapEntry; - static void dump_map_entry(OutputFormat output_format, MapEntry *e, MapEntry *next) { switch (output_format) { case OFORMAT_HUMAN: - if ((e->flags & BDRV_BLOCK_DATA) && - !(e->flags & BDRV_BLOCK_OFFSET_VALID)) { + if (e->data && !e->has_offset) { error_report("File contains external, encrypted or compressed clusters."); exit(1); } - if ((e->flags & (BDRV_BLOCK_DATA|BDRV_BLOCK_ZERO)) == BDRV_BLOCK_DATA) { + if (e->data && !e->zero) { printf("%#-16"PRIx64"%#-16"PRIx64"%#-16"PRIx64"%s\n", - e->start, e->length, e->offset, e->bs->filename); + e->start, e->length, + e->has_offset ? e->offset : 0, + e->has_filename ? e->filename : ""); } /* This format ignores the distinction between 0, ZERO and ZERO|DATA. * Modify the flags here to allow more coalescing. */ - if (next && - (next->flags & (BDRV_BLOCK_DATA|BDRV_BLOCK_ZERO)) != BDRV_BLOCK_DATA) { - next->flags &= ~BDRV_BLOCK_DATA; - next->flags |= BDRV_BLOCK_ZERO; + if (next && (!next->data || next->zero)) { + next->data = false; + next->zero = true; } break; case OFORMAT_JSON: - printf("%s{ \"start\": %"PRId64", \"length\": %"PRId64", \"depth\": %d," - " \"zero\": %s, \"data\": %s", + printf("%s{ \"start\": %"PRId64", \"length\": %"PRId64"," + " \"depth\": %"PRId64", \"zero\": %s, \"data\": %s", (e->start == 0 ? "[" : ",\n"), e->start, e->length, e->depth, - (e->flags & BDRV_BLOCK_ZERO) ? "true" : "false", - (e->flags & BDRV_BLOCK_DATA) ? "true" : "false"); - if (e->flags & BDRV_BLOCK_OFFSET_VALID) { + e->zero ? "true" : "false", + e->data ? "true" : "false"); + if (e->has_offset) { printf(", \"offset\": %"PRId64"", e->offset); } putchar('}'); @@ -2233,13 +2223,39 @@ static int get_block_status(BlockDriverState *bs, int64_t sector_num, e->start = sector_num * BDRV_SECTOR_SIZE; e->length = nb_sectors * BDRV_SECTOR_SIZE; - e->flags = ret & ~BDRV_BLOCK_OFFSET_MASK; + e->data = !!(ret & BDRV_BLOCK_DATA); + e->zero = !!(ret & BDRV_BLOCK_ZERO); e->offset = ret & BDRV_BLOCK_OFFSET_MASK; + e->has_offset = !!(ret & BDRV_BLOCK_OFFSET_VALID); e->depth = depth; - e->bs = file; + if (file && e->has_offset) { + e->has_filename = true; + e->filename = file->filename; + } return 0; } +static inline bool entry_mergeable(const MapEntry *curr, const MapEntry *next) +{ + if (curr->length == 0) { + return false; + } + if (curr->zero != next->zero || + curr->data != next->data || + curr->depth != next->depth || + curr->has_filename != next->has_filename || + curr->has_offset != next->has_offset) { + return false; + } + if (curr->has_filename && strcmp(curr->filename, next->filename)) { + return false; + } + if (curr->has_offset && curr->offset + curr->length != next->offset) { + return false; + } + return true; +} + static int img_map(int argc, char **argv) { int c; @@ -2321,10 +2337,7 @@ static int img_map(int argc, char **argv) goto out; } - if (curr.length != 0 && curr.flags == next.flags && - curr.depth == next.depth && - ((curr.flags & BDRV_BLOCK_OFFSET_VALID) == 0 || - curr.offset + curr.length == next.offset)) { + if (entry_mergeable(&curr, &next)) { curr.length += next.length; continue; } -- cgit v1.1 From c7fc50d37620550c52854ca15c2a18da1b8155e0 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Tue, 26 Jan 2016 11:59:03 +0800 Subject: iotests: Add "qemu-img map" test for VMDK extents Signed-off-by: Fam Zheng Message-id: 1453780743-16806-17-git-send-email-famz@redhat.com Reviewed-by: Max Reitz Signed-off-by: Max Reitz --- tests/qemu-iotests/059 | 10 ++++++++++ tests/qemu-iotests/059.out | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059 index 0ded0c3..0332bbb 100755 --- a/tests/qemu-iotests/059 +++ b/tests/qemu-iotests/059 @@ -133,6 +133,16 @@ $QEMU_IO -c "write -P 0xa 900G 512" "$TEST_IMG" | _filter_qemu_io $QEMU_IO -c "read -v 900G 1024" "$TEST_IMG" | _filter_qemu_io echo +echo "=== Testing qemu-img map on extents ===" +for fmt in monolithicSparse twoGbMaxExtentSparse; do + IMGOPTS="subformat=$fmt" _make_test_img 31G + $QEMU_IO -c "write 65024 1k" "$TEST_IMG" | _filter_qemu_io + $QEMU_IO -c "write 2147483136 1k" "$TEST_IMG" | _filter_qemu_io + $QEMU_IO -c "write 5G 1k" "$TEST_IMG" | _filter_qemu_io + $QEMU_IMG map "$TEST_IMG" | _filter_testdir +done + +echo echo "=== Testing afl image with a very large capacity ===" _use_sample_img afl9.vmdk.bz2 _img_info diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out index 9d506cb..678adb4 100644 --- a/tests/qemu-iotests/059.out +++ b/tests/qemu-iotests/059.out @@ -2335,6 +2335,31 @@ e1000003f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ read 1024/1024 bytes at offset 966367641600 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +=== Testing qemu-img map on extents === +Formatting 'TEST_DIR/iotest-version3.IMGFMT', fmt=IMGFMT size=33285996544 subformat=monolithicSparse +wrote 1024/1024 bytes at offset 65024 +1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 1024/1024 bytes at offset 2147483136 +1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 1024/1024 bytes at offset 5368709120 +1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +Offset Length Mapped to File +0 0x20000 0x3f0000 TEST_DIR/iotest-version3.vmdk +0x7fff0000 0x20000 0x410000 TEST_DIR/iotest-version3.vmdk +0x140000000 0x10000 0x430000 TEST_DIR/iotest-version3.vmdk +Formatting 'TEST_DIR/iotest-version3.IMGFMT', fmt=IMGFMT size=33285996544 subformat=twoGbMaxExtentSparse +wrote 1024/1024 bytes at offset 65024 +1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 1024/1024 bytes at offset 2147483136 +1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 1024/1024 bytes at offset 5368709120 +1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +Offset Length Mapped to File +0 0x20000 0x50000 TEST_DIR/iotest-version3-s001.vmdk +0x7fff0000 0x10000 0x70000 TEST_DIR/iotest-version3-s001.vmdk +0x80000000 0x10000 0x50000 TEST_DIR/iotest-version3-s002.vmdk +0x140000000 0x10000 0x50000 TEST_DIR/iotest-version3-s003.vmdk + === Testing afl image with a very large capacity === qemu-img: Can't get size of device 'image': File too large *** done -- cgit v1.1 From f8aa905a4fec89863c82de4186352447d851871e Mon Sep 17 00:00:00 2001 From: Jeff Cody Date: Mon, 1 Feb 2016 20:33:10 -0500 Subject: block: set device_list.tqe_prev to NULL on BDS removal This fixes a regression introduced with commit 3f09bfbc7. Multiple bugs arise in conjunction with live snapshots and mirroring operations (which include active layer commit). After a live snapshot occurs, the active layer and the base layer both have a non-NULL tqe_prev field in the device_list, although the base node's tqe_prev field points to a NULL entry. This non-NULL tqe_prev field occurs after the bdrv_append() in the external snapshot calls change_parent_backing_link(). In change_parent_backing_link(), when the previous active layer is removed from device_list, the device_list.tqe_prev pointer is not set to NULL. The operating scheme in the block layer is to indicate that a BDS belongs in the bdrv_states device_list iff the device_list.tqe_prev pointer is non-NULL. This patch does two things: 1.) Introduces a new block layer helper bdrv_device_remove() to remove a BDS from the device_list, and 2.) uses that new API, which also fixes the regression once used in change_parent_backing_link(). Signed-off-by: Jeff Cody Message-id: 0cd51e11c0666c04ddb7c05293fe94afeb551e89.1454376655.git.jcody@redhat.com Reviewed-by: Max Reitz Signed-off-by: Max Reitz --- block.c | 24 ++++++++++++++---------- blockdev.c | 3 +-- include/block/block.h | 1 + 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/block.c b/block.c index ff1aafc..dff3a3a 100644 --- a/block.c +++ b/block.c @@ -2242,21 +2242,25 @@ void bdrv_close_all(void) } } +/* Note that bs->device_list.tqe_prev is initially null, + * and gets set to non-null by QTAILQ_INSERT_TAIL(). Establish + * the useful invariant "bs in bdrv_states iff bs->tqe_prev" by + * resetting it to null on remove. */ +void bdrv_device_remove(BlockDriverState *bs) +{ + QTAILQ_REMOVE(&bdrv_states, bs, device_list); + bs->device_list.tqe_prev = NULL; +} + /* make a BlockDriverState anonymous by removing from bdrv_state and * graph_bdrv_state list. Also, NULL terminate the device_name to prevent double remove */ void bdrv_make_anon(BlockDriverState *bs) { - /* - * Take care to remove bs from bdrv_states only when it's actually - * in it. Note that bs->device_list.tqe_prev is initially null, - * and gets set to non-null by QTAILQ_INSERT_TAIL(). Establish - * the useful invariant "bs in bdrv_states iff bs->tqe_prev" by - * resetting it to null on remove. - */ + /* Take care to remove bs from bdrv_states only when it's actually + * in it. */ if (bs->device_list.tqe_prev) { - QTAILQ_REMOVE(&bdrv_states, bs, device_list); - bs->device_list.tqe_prev = NULL; + bdrv_device_remove(bs); } if (bs->node_name[0] != '\0') { QTAILQ_REMOVE(&graph_bdrv_states, bs, node_list); @@ -2297,7 +2301,7 @@ static void change_parent_backing_link(BlockDriverState *from, if (!to->device_list.tqe_prev) { QTAILQ_INSERT_BEFORE(from, to, device_list); } - QTAILQ_REMOVE(&bdrv_states, from, device_list); + bdrv_device_remove(from); } } diff --git a/blockdev.c b/blockdev.c index 35e1e5c..be4ca44 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2408,8 +2408,7 @@ void qmp_x_blockdev_remove_medium(const char *device, Error **errp) /* This follows the convention established by bdrv_make_anon() */ if (bs->device_list.tqe_prev) { - QTAILQ_REMOVE(&bdrv_states, bs, device_list); - bs->device_list.tqe_prev = NULL; + bdrv_device_remove(bs); } blk_remove_bs(blk); diff --git a/include/block/block.h b/include/block/block.h index 0035ad8..1c4f4d8 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -199,6 +199,7 @@ int bdrv_create(BlockDriver *drv, const char* filename, int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp); BlockDriverState *bdrv_new_root(void); BlockDriverState *bdrv_new(void); +void bdrv_device_remove(BlockDriverState *bs); void bdrv_make_anon(BlockDriverState *bs); void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top); void bdrv_replace_in_backing_chain(BlockDriverState *old, -- cgit v1.1 From 8983b670f62ab5e5e8dd2690bf8304123651bfe5 Mon Sep 17 00:00:00 2001 From: Jeff Cody Date: Mon, 1 Feb 2016 20:33:11 -0500 Subject: block: qemu-iotests - add test for snapshot, commit, snapshot bug Signed-off-by: Jeff Cody Message-id: 2dbc05efba2f683cb3aaf71aaa9b776ebf7ec57c.1454376655.git.jcody@redhat.com Reviewed-by: Max Reitz [Moved test number from 143 to 144] Signed-off-by: Max Reitz --- tests/qemu-iotests/144 | 114 +++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/144.out | 24 ++++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 139 insertions(+) create mode 100755 tests/qemu-iotests/144 create mode 100644 tests/qemu-iotests/144.out diff --git a/tests/qemu-iotests/144 b/tests/qemu-iotests/144 new file mode 100755 index 0000000..00de3c33 --- /dev/null +++ b/tests/qemu-iotests/144 @@ -0,0 +1,114 @@ +#!/bin/bash +# Check live snapshot, followed by active commit, and another snapshot. +# +# This test is to catch the error case of BZ #1300209: +# https://bugzilla.redhat.com/show_bug.cgi?id=1300209 +# +# Copyright (C) 2016 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +# creator +owner=jcody@redhat.com + +seq=`basename $0` +echo "QA output created by $seq" + +here=`pwd` +status=1 # failure is the default! + +TMP_SNAP1=${TEST_DIR}/tmp.qcow2 +TMP_SNAP2=${TEST_DIR}/tmp2.qcow2 + +_cleanup() +{ + _cleanup_qemu + rm -f "${TEST_IMG}" "${TMP_SNAP1}" "${TMP_SNAP2}" +} + +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter +. ./common.qemu + +_supported_fmt qcow2 +_supported_proto file +_supported_os Linux + +size=512M + +_make_test_img $size + +echo +echo === Launching QEMU === +echo + +qemu_comm_method="qmp" +_launch_qemu -drive file="${TEST_IMG}",if=virtio +h=$QEMU_HANDLE + + +echo +echo === Performing Live Snapshot 1 === +echo + +_send_qemu_cmd $h "{ 'execute': 'qmp_capabilities' }" "return" + + +# First live snapshot, new overlay as active layer +_send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot-sync', + 'arguments': { + 'device': 'virtio0', + 'snapshot-file':'${TMP_SNAP1}', + 'format': 'qcow2' + } + }" "return" + +echo +echo === Performing block-commit on active layer === +echo + +# Block commit on active layer, push the new overlay into base +_send_qemu_cmd $h "{ 'execute': 'block-commit', + 'arguments': { + 'device': 'virtio0' + } + }" "READY" + +_send_qemu_cmd $h "{ 'execute': 'block-job-complete', + 'arguments': { + 'device': 'virtio0' + } + }" "COMPLETED" + +echo +echo === Performing Live Snapshot 2 === +echo + +# New live snapshot, new overlays as active layer +_send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot-sync', + 'arguments': { + 'device': 'virtio0', + 'snapshot-file':'${TMP_SNAP2}', + 'format': 'qcow2' + } + }" "return" + +# success, all done +echo "*** done" +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/144.out b/tests/qemu-iotests/144.out new file mode 100644 index 0000000..410d741 --- /dev/null +++ b/tests/qemu-iotests/144.out @@ -0,0 +1,24 @@ +QA output created by 144 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=536870912 + +=== Launching QEMU === + + +=== Performing Live Snapshot 1 === + +{"return": {}} +Formatting 'TEST_DIR/tmp.qcow2', fmt=qcow2 size=536870912 backing_file=TEST_DIR/t.qcow2 backing_fmt=qcow2 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16 +{"return": {}} + +=== Performing block-commit on active layer === + +{"return": {}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "virtio0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}} +{"return": {}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "virtio0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}} + +=== Performing Live Snapshot 2 === + +Formatting 'TEST_DIR/tmp2.qcow2', fmt=qcow2 size=536870912 backing_file=TEST_DIR/t.qcow2 backing_fmt=qcow2 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16 +{"return": {}} +*** done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index cbc970a..65df029 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -146,3 +146,4 @@ 141 rw auto quick 142 auto 143 auto quick +144 rw auto quick -- cgit v1.1