From b53169eae06d6cf6f512a8b1fdd5424a0a6aab85 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 26 Jun 2013 14:11:57 +0200 Subject: [PATCH 1/8] blockdev: add sync mode to drive-backup QMP command The drive-backup command is similar to the drive-mirror command, except no guest data written after the command executes gets copied. Add a sync mode argument which determines whether the entire disk is copied, just allocated clusters, or only clusters being written to by the guest. Currently only sync mode 'full' is supported - it copies the entire disk. For read-only point-in-time snapshots we may only need sync mode 'none' since the target can be a qcow2 file using the guest's disk as its backing file (no need to copy the entire disk). Finally, sync mode 'top' is useful if we wish to preserve the backing chain. Note that this patch just adds the sync mode argument to drive-backup. It does not implement sync modes 'top' or 'none'. This patch is necessary so we can add a drive-backup HMP command that behaves like the existing drive-mirror HMP command and takes a sync mode. Signed-off-by: Stefan Hajnoczi Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- blockdev.c | 6 ++++++ qapi-schema.json | 7 ++++++- qmp-commands.hx | 6 +++++- tests/qemu-iotests/055 | 36 +++++++++++++++++++++--------------- 4 files changed, 38 insertions(+), 17 deletions(-) diff --git a/blockdev.c b/blockdev.c index b3a57e0c1d..c5abd65182 100644 --- a/blockdev.c +++ b/blockdev.c @@ -936,6 +936,7 @@ static void drive_backup_prepare(BlkTransactionState *common, Error **errp) qmp_drive_backup(backup->device, backup->target, backup->has_format, backup->format, + backup->sync, backup->has_mode, backup->mode, backup->has_speed, backup->speed, backup->has_on_source_error, backup->on_source_error, @@ -1421,6 +1422,7 @@ void qmp_block_commit(const char *device, void qmp_drive_backup(const char *device, const char *target, bool has_format, const char *format, + enum MirrorSyncMode sync, bool has_mode, enum NewImageMode mode, bool has_speed, int64_t speed, bool has_on_source_error, BlockdevOnError on_source_error, @@ -1435,6 +1437,10 @@ void qmp_drive_backup(const char *device, const char *target, int64_t size; int ret; + if (sync != MIRROR_SYNC_MODE_FULL) { + error_setg(errp, "only sync mode 'full' is currently supported"); + return; + } if (!has_speed) { speed = 0; } diff --git a/qapi-schema.json b/qapi-schema.json index b251d282cc..cf57783caf 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1634,6 +1634,10 @@ # @format: #optional the format of the new destination, default is to # probe if @mode is 'existing', else the format of the source # +# @sync: what parts of the disk image should be copied to the destination +# (all the disk, only the sectors allocated in the topmost image, or +# only new I/O). +# # @mode: #optional whether and how QEMU should create a new image, default is # 'absolute-paths'. # @@ -1655,7 +1659,8 @@ ## { 'type': 'DriveBackup', 'data': { 'device': 'str', 'target': 'str', '*format': 'str', - '*mode': 'NewImageMode', '*speed': 'int', + 'sync': 'MirrorSyncMode', '*mode': 'NewImageMode', + '*speed': 'int', '*on-source-error': 'BlockdevOnError', '*on-target-error': 'BlockdevOnError' } } diff --git a/qmp-commands.hx b/qmp-commands.hx index 362f0e1ef8..e075df423a 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -913,7 +913,7 @@ EQMP { .name = "drive-backup", - .args_type = "device:B,target:s,speed:i?,mode:s?,format:s?," + .args_type = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?," "on-source-error:s?,on-target-error:s?", .mhandler.cmd_new = qmp_marshal_input_drive_backup, }, @@ -939,6 +939,10 @@ Arguments: - "format": the format of the new destination, default is to probe if 'mode' is 'existing', else the format of the source (json-string, optional) +- "sync": what parts of the disk image should be copied to the destination; + possibilities include "full" for all the disk, "top" for only the sectors + allocated in the topmost image, or "none" to only replicate new I/O + (MirrorSyncMode). - "mode": whether and how QEMU should create a new image (NewImageMode, optional, default 'absolute-paths') - "speed": the maximum speed, in bytes per second (json-int, optional) diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055 index 887c959a3d..c66f8dbd7d 100755 --- a/tests/qemu-iotests/055 +++ b/tests/qemu-iotests/055 @@ -54,7 +54,7 @@ class TestSingleDrive(iotests.QMPTestCase): self.assert_no_active_block_jobs() result = self.vm.qmp('drive-backup', device='drive0', - target=target_img) + target=target_img, sync='full') self.assert_qmp(result, 'return', {}) event = self.cancel_and_wait() @@ -64,7 +64,7 @@ class TestSingleDrive(iotests.QMPTestCase): self.assert_no_active_block_jobs() result = self.vm.qmp('drive-backup', device='drive0', - target=target_img) + target=target_img, sync='full') self.assert_qmp(result, 'return', {}) result = self.vm.qmp('block-job-pause', device='drive0') @@ -89,17 +89,17 @@ class TestSingleDrive(iotests.QMPTestCase): def test_medium_not_found(self): result = self.vm.qmp('drive-backup', device='ide1-cd0', - target=target_img) + target=target_img, sync='full') self.assert_qmp(result, 'error/class', 'GenericError') def test_image_not_found(self): result = self.vm.qmp('drive-backup', device='drive0', - mode='existing', target=target_img) + target=target_img, sync='full', mode='existing') self.assert_qmp(result, 'error/class', 'GenericError') def test_device_not_found(self): result = self.vm.qmp('drive-backup', device='nonexistent', - target=target_img) + target=target_img, sync='full') self.assert_qmp(result, 'error/class', 'DeviceNotFound') class TestSetSpeed(iotests.QMPTestCase): @@ -119,7 +119,7 @@ class TestSetSpeed(iotests.QMPTestCase): self.assert_no_active_block_jobs() result = self.vm.qmp('drive-backup', device='drive0', - target=target_img) + target=target_img, sync='full') self.assert_qmp(result, 'return', {}) # Default speed is 0 @@ -140,7 +140,7 @@ class TestSetSpeed(iotests.QMPTestCase): # Check setting speed in drive-backup works result = self.vm.qmp('drive-backup', device='drive0', - target=target_img, speed=4*1024*1024) + target=target_img, sync='full', speed=4*1024*1024) self.assert_qmp(result, 'return', {}) result = self.vm.qmp('query-block-jobs') @@ -154,13 +154,13 @@ class TestSetSpeed(iotests.QMPTestCase): self.assert_no_active_block_jobs() result = self.vm.qmp('drive-backup', device='drive0', - target=target_img, speed=-1) + target=target_img, sync='full', speed=-1) self.assert_qmp(result, 'error/class', 'GenericError') self.assert_no_active_block_jobs() result = self.vm.qmp('drive-backup', device='drive0', - target=target_img) + target=target_img, sync='full') self.assert_qmp(result, 'return', {}) result = self.vm.qmp('block-job-set-speed', device='drive0', speed=-1) @@ -196,7 +196,8 @@ class TestSingleTransaction(iotests.QMPTestCase): result = self.vm.qmp('transaction', actions=[{ 'type': 'drive-backup', 'data': { 'device': 'drive0', - 'target': target_img }, + 'target': target_img, + 'sync': 'full' }, } ]) self.assert_qmp(result, 'return', {}) @@ -210,7 +211,8 @@ class TestSingleTransaction(iotests.QMPTestCase): result = self.vm.qmp('transaction', actions=[{ 'type': 'drive-backup', 'data': { 'device': 'drive0', - 'target': target_img }, + 'target': target_img, + 'sync': 'full' }, } ]) self.assert_qmp(result, 'return', {}) @@ -239,7 +241,8 @@ class TestSingleTransaction(iotests.QMPTestCase): result = self.vm.qmp('transaction', actions=[{ 'type': 'drive-backup', 'data': { 'device': 'ide1-cd0', - 'target': target_img }, + 'target': target_img, + 'sync': 'full' }, } ]) self.assert_qmp(result, 'error/class', 'GenericError') @@ -249,7 +252,8 @@ class TestSingleTransaction(iotests.QMPTestCase): 'type': 'drive-backup', 'data': { 'device': 'drive0', 'mode': 'existing', - 'target': target_img }, + 'target': target_img, + 'sync': 'full' }, } ]) self.assert_qmp(result, 'error/class', 'GenericError') @@ -259,7 +263,8 @@ class TestSingleTransaction(iotests.QMPTestCase): 'type': 'drive-backup', 'data': { 'device': 'nonexistent', 'mode': 'existing', - 'target': target_img }, + 'target': target_img, + 'sync': 'full' }, } ]) self.assert_qmp(result, 'error/class', 'DeviceNotFound') @@ -269,7 +274,8 @@ class TestSingleTransaction(iotests.QMPTestCase): 'type': 'drive-backup', 'data': { 'device': 'nonexistent', 'mode': 'existing', - 'target': target_img }, + 'target': target_img, + 'sync': 'full' }, }, { 'type': 'Abort', 'data': {}, From de90930a0c45760e7523138fac57ff07312bf51d Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 26 Jun 2013 14:11:58 +0200 Subject: [PATCH 2/8] block: add drive_backup HMP command Make "drive_backup" available on the HMP monitor: drive_backup [-n] [-f] device target [format] The -n flag requests QEMU to reuse the image found in new-image-file, instead of recreating it from scratch. The -f flag requests QEMU to copy the whole disk, so that the result does not need a backing file. Note that this flag *must* currently be passed since the other sync modes ('none' and 'top') have not been implemented yet. Requiring it ensures that "drive_backup" behaves like "drive_mirror". Signed-off-by: Stefan Hajnoczi Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- hmp-commands.hx | 20 ++++++++++++++++++++ hmp.c | 28 ++++++++++++++++++++++++++++ hmp.h | 1 + 3 files changed, 49 insertions(+) diff --git a/hmp-commands.hx b/hmp-commands.hx index d1cdcfb71b..8c6b91a9c7 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1056,6 +1056,26 @@ STEXI @findex drive_mirror Start mirroring a block device's writes to a new destination, using the specified target. +ETEXI + + { + .name = "drive_backup", + .args_type = "reuse:-n,full:-f,device:B,target:s,format:s?", + .params = "[-n] [-f] device target [format]", + .help = "initiates a point-in-time\n\t\t\t" + "copy for a device. The device's contents are\n\t\t\t" + "copied to the new image file, excluding data that\n\t\t\t" + "is written after the command is started.\n\t\t\t" + "The -n flag requests QEMU to reuse the image found\n\t\t\t" + "in new-image-file, instead of recreating it from scratch.\n\t\t\t" + "The -f flag requests QEMU to copy the whole disk,\n\t\t\t" + "so that the result does not need a backing file.\n\t\t\t", + .mhandler.cmd = hmp_drive_backup, + }, +STEXI +@item drive_backup +@findex drive_backup +Start a point-in-time copy of a block device to a specificed target. ETEXI { diff --git a/hmp.c b/hmp.c index 2daed430d6..dc4d8d453f 100644 --- a/hmp.c +++ b/hmp.c @@ -907,6 +907,34 @@ void hmp_drive_mirror(Monitor *mon, const QDict *qdict) hmp_handle_error(mon, &errp); } +void hmp_drive_backup(Monitor *mon, const QDict *qdict) +{ + const char *device = qdict_get_str(qdict, "device"); + const char *filename = qdict_get_str(qdict, "target"); + const char *format = qdict_get_try_str(qdict, "format"); + int reuse = qdict_get_try_bool(qdict, "reuse", 0); + int full = qdict_get_try_bool(qdict, "full", 0); + enum NewImageMode mode; + Error *errp = NULL; + + if (!filename) { + error_set(&errp, QERR_MISSING_PARAMETER, "target"); + hmp_handle_error(mon, &errp); + return; + } + + if (reuse) { + mode = NEW_IMAGE_MODE_EXISTING; + } else { + mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS; + } + + qmp_drive_backup(device, filename, !!format, format, + full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP, + true, mode, false, 0, false, 0, false, 0, &errp); + hmp_handle_error(mon, &errp); +} + void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict) { const char *device = qdict_get_str(qdict, "device"); diff --git a/hmp.h b/hmp.h index 56d2e92aa6..6c3bdcd4c2 100644 --- a/hmp.h +++ b/hmp.h @@ -55,6 +55,7 @@ void hmp_balloon(Monitor *mon, const QDict *qdict); void hmp_block_resize(Monitor *mon, const QDict *qdict); void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict); void hmp_drive_mirror(Monitor *mon, const QDict *qdict); +void hmp_drive_backup(Monitor *mon, const QDict *qdict); void hmp_migrate_cancel(Monitor *mon, const QDict *qdict); void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict); void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict); From 98289620e0460fa595581020ab20127da4a2fc44 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 10 Jul 2013 15:47:39 +0200 Subject: [PATCH 3/8] block: Don't parse protocol from file.filename One of the major reasons for doing something new for -blockdev and blockdev-add was that the old block layer code parses filenames instead of just taking them literally. So we should really leave it untouched when it's passing using the new interfaces (like -drive file.filename=...). This allows opening relative file names that contain a colon. Signed-off-by: Kevin Wolf Reviewed-by: Eric Blake --- block.c | 17 ++++++++++++----- block/sheepdog.c | 2 +- include/block/block.h | 3 ++- qemu-img.c | 4 ++-- tests/qemu-iotests/051 | 12 ++++++++++++ tests/qemu-iotests/051.out | 14 ++++++++++++++ 6 files changed, 43 insertions(+), 9 deletions(-) diff --git a/block.c b/block.c index 183fec8aa5..2df65c88bf 100644 --- a/block.c +++ b/block.c @@ -417,7 +417,7 @@ int bdrv_create_file(const char* filename, QEMUOptionParameter *options) { BlockDriver *drv; - drv = bdrv_find_protocol(filename); + drv = bdrv_find_protocol(filename, true); if (drv == NULL) { return -ENOENT; } @@ -482,7 +482,8 @@ static BlockDriver *find_hdev_driver(const char *filename) return drv; } -BlockDriver *bdrv_find_protocol(const char *filename) +BlockDriver *bdrv_find_protocol(const char *filename, + bool allow_protocol_prefix) { BlockDriver *drv1; char protocol[128]; @@ -503,9 +504,10 @@ BlockDriver *bdrv_find_protocol(const char *filename) return drv1; } - if (!path_has_protocol(filename)) { + if (!path_has_protocol(filename) || !allow_protocol_prefix) { return bdrv_find_format("file"); } + p = strchr(filename, ':'); assert(p != NULL); len = p - filename; @@ -784,6 +786,7 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, BlockDriverState *bs; BlockDriver *drv; const char *drvname; + bool allow_protocol_prefix = false; int ret; /* NULL means an empty set of options */ @@ -800,6 +803,7 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, filename = qdict_get_try_str(options, "filename"); } else if (filename && !qdict_haskey(options, "filename")) { qdict_put(options, "filename", qstring_from_str(filename)); + allow_protocol_prefix = true; } else { qerror_report(ERROR_CLASS_GENERIC_ERROR, "Can't specify 'file' and " "'filename' options at the same time"); @@ -813,7 +817,10 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename, drv = bdrv_find_whitelisted_format(drvname, !(flags & BDRV_O_RDWR)); qdict_del(options, "driver"); } else if (filename) { - drv = bdrv_find_protocol(filename); + drv = bdrv_find_protocol(filename, allow_protocol_prefix); + if (!drv) { + qerror_report(ERROR_CLASS_GENERIC_ERROR, "Unknown protocol"); + } } else { qerror_report(ERROR_CLASS_GENERIC_ERROR, "Must specify either driver or file"); @@ -4452,7 +4459,7 @@ void bdrv_img_create(const char *filename, const char *fmt, return; } - proto_drv = bdrv_find_protocol(filename); + proto_drv = bdrv_find_protocol(filename, true); if (!proto_drv) { error_setg(errp, "Unknown protocol '%s'", filename); return; diff --git a/block/sheepdog.c b/block/sheepdog.c index b397b5b4d3..6a41ad9b3c 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -1510,7 +1510,7 @@ static int sd_create(const char *filename, QEMUOptionParameter *options) BlockDriver *drv; /* Currently, only Sheepdog backing image is supported. */ - drv = bdrv_find_protocol(backing_file); + drv = bdrv_find_protocol(backing_file, true); if (!drv || strcmp(drv->protocol_name, "sheepdog") != 0) { error_report("backing_file must be a sheepdog image"); ret = -EINVAL; diff --git a/include/block/block.h b/include/block/block.h index dd8eca1be1..eeb48162af 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -111,7 +111,8 @@ bool bdrv_io_limits_enabled(BlockDriverState *bs); void bdrv_init(void); void bdrv_init_with_whitelist(void); -BlockDriver *bdrv_find_protocol(const char *filename); +BlockDriver *bdrv_find_protocol(const char *filename, + bool allow_protocol_prefix); BlockDriver *bdrv_find_format(const char *format_name); BlockDriver *bdrv_find_whitelisted_format(const char *format_name, bool readonly); diff --git a/qemu-img.c b/qemu-img.c index f8c97d34d1..c55ca5c557 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -240,7 +240,7 @@ static int print_block_option_help(const char *filename, const char *fmt) return 1; } - proto_drv = bdrv_find_protocol(filename); + proto_drv = bdrv_find_protocol(filename, true); if (!proto_drv) { error_report("Unknown protocol '%s'", filename); return 1; @@ -1261,7 +1261,7 @@ static int img_convert(int argc, char **argv) goto out; } - proto_drv = bdrv_find_protocol(out_filename); + proto_drv = bdrv_find_protocol(out_filename, true); if (!proto_drv) { error_report("Unknown protocol '%s'", out_filename); ret = -1; diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051 index 8039e23ab3..1cf8bf79b6 100755 --- a/tests/qemu-iotests/051 +++ b/tests/qemu-iotests/051 @@ -149,6 +149,18 @@ echo run_qemu -drive file=$TEST_IMG,file.driver=file run_qemu -drive file=$TEST_IMG,file.driver=qcow2 +echo +echo === Parsing protocol from file name === +echo + +# Protocol strings are supposed to be parsed from traditional option strings, +# but not when using driver-specific options. We can distinguish them by the +# error message for non-existing files. + +run_qemu -hda foo:bar +run_qemu -drive file=foo:bar +run_qemu -drive file.filename=foo:bar + # success, all done echo "*** done" rm -f $seq.full diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out index 3d1ac7b7da..6b3d63612e 100644 --- a/tests/qemu-iotests/051.out +++ b/tests/qemu-iotests/051.out @@ -169,4 +169,18 @@ Testing: -drive file=TEST_DIR/t.qcow2,file.driver=qcow2 QEMU_PROG: -drive file=TEST_DIR/t.qcow2,file.driver=qcow2: Can't use 'qcow2' as a block driver for the protocol level QEMU_PROG: -drive file=TEST_DIR/t.qcow2,file.driver=qcow2: could not open disk image TEST_DIR/t.qcow2: Invalid argument + +=== Parsing protocol from file name === + +Testing: -hda foo:bar +QEMU_PROG: -hda foo:bar: Unknown protocol +QEMU_PROG: -hda foo:bar: could not open disk image foo:bar: No such file or directory + +Testing: -drive file=foo:bar +QEMU_PROG: -drive file=foo:bar: Unknown protocol +QEMU_PROG: -drive file=foo:bar: could not open disk image foo:bar: No such file or directory + +Testing: -drive file.filename=foo:bar +QEMU_PROG: -drive file.filename=foo:bar: could not open disk image ide0-hd0: No such file or directory + *** done From 7a370406bdd13b1d46230d1cbca308d984d0dcae Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Wed, 10 Jul 2013 17:30:26 +0200 Subject: [PATCH 4/8] qemu-iotests: Update 051 reference output This has been broken by commit bd5c51ee. Signed-off-by: Kevin Wolf --- tests/qemu-iotests/051.out | 107 ++++++++++++++++++++++--------- tests/qemu-iotests/common.filter | 2 +- 2 files changed, 76 insertions(+), 33 deletions(-) diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out index 6b3d63612e..95ff245f40 100644 --- a/tests/qemu-iotests/051.out +++ b/tests/qemu-iotests/051.out @@ -23,10 +23,12 @@ QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=foo: could not === Enable and disable lazy refcounting on the command line, plus some invalid values === Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy_refcounts=on -qququiquit +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) qququiquit Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy_refcounts=off -qququiquit +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) qququiquit Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy_refcounts= QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy_refcounts=: Parameter 'lazy_refcounts' expects 'on' or 'off' @@ -49,112 +51,152 @@ QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy_refcounts=on: Lazy ref QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy_refcounts=on: could not open disk image TEST_DIR/t.qcow2: Invalid argument Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy_refcounts=off -qququiquit +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) qququiquit === No medium === Testing: -drive if=floppy -qququiquit +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) qququiquit Testing: -drive if=ide,media=cdrom -qququiquit +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) qququiquit Testing: -drive if=scsi,media=cdrom -qququiquit +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) qququiquit Testing: -drive if=ide -QEMU_PROG: Device needs media, but drive is empty +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) QEMU_PROG: Device needs media, but drive is empty +QEMU_PROG: Device initialization failed. QEMU_PROG: Initialization of device ide-hd failed Testing: -drive if=virtio -QEMU_PROG: -drive if=virtio: Device needs media, but drive is empty +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) QEMU_PROG: -drive if=virtio: Device needs media, but drive is empty +QEMU_PROG: -drive if=virtio: Device initialization failed. +QEMU_PROG: -drive if=virtio: Device initialization failed. QEMU_PROG: -drive if=virtio: Device 'virtio-blk-pci' could not be initialized Testing: -drive if=scsi -QEMU_PROG: -drive if=scsi: Device needs media, but drive is empty +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) QEMU_PROG: -drive if=scsi: Device needs media, but drive is empty +QEMU_PROG: -drive if=scsi: Device initialization failed. +QEMU_PROG: Device initialization failed. QEMU_PROG: Initialization of device lsi53c895a failed Testing: -drive if=none,id=disk -device ide-cd,drive=disk -qququiquit +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) qququiquit Testing: -drive if=none,id=disk -device lsi53c895a -device scsi-cd,drive=disk -qququiquit +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) qququiquit Testing: -drive if=none,id=disk -device ide-drive,drive=disk -QEMU_PROG: -device ide-drive,drive=disk: Device needs media, but drive is empty +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) QEMU_PROG: -device ide-drive,drive=disk: Device needs media, but drive is empty +QEMU_PROG: -device ide-drive,drive=disk: Device initialization failed. QEMU_PROG: -device ide-drive,drive=disk: Device 'ide-drive' could not be initialized Testing: -drive if=none,id=disk -device ide-hd,drive=disk -QEMU_PROG: -device ide-hd,drive=disk: Device needs media, but drive is empty +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) QEMU_PROG: -device ide-hd,drive=disk: Device needs media, but drive is empty +QEMU_PROG: -device ide-hd,drive=disk: Device initialization failed. QEMU_PROG: -device ide-hd,drive=disk: Device 'ide-hd' could not be initialized Testing: -drive if=none,id=disk -device lsi53c895a -device scsi-disk,drive=disk -QEMU_PROG: -device scsi-disk,drive=disk: Device needs media, but drive is empty +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) QEMU_PROG: -device scsi-disk,drive=disk: Device needs media, but drive is empty +QEMU_PROG: -device scsi-disk,drive=disk: Device initialization failed. QEMU_PROG: -device scsi-disk,drive=disk: Device 'scsi-disk' could not be initialized Testing: -drive if=none,id=disk -device lsi53c895a -device scsi-hd,drive=disk -QEMU_PROG: -device scsi-hd,drive=disk: Device needs media, but drive is empty +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) QEMU_PROG: -device scsi-hd,drive=disk: Device needs media, but drive is empty +QEMU_PROG: -device scsi-hd,drive=disk: Device initialization failed. QEMU_PROG: -device scsi-hd,drive=disk: Device 'scsi-hd' could not be initialized === Read-only === Testing: -drive file=TEST_DIR/t.qcow2,if=floppy,readonly=on -qququiquit +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) qququiquit Testing: -drive file=TEST_DIR/t.qcow2,if=ide,media=cdrom,readonly=on -qququiquit +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) qququiquit Testing: -drive file=TEST_DIR/t.qcow2,if=scsi,media=cdrom,readonly=on -qququiquit +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) qququiquit Testing: -drive file=TEST_DIR/t.qcow2,if=ide,readonly=on QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=ide,readonly=on: readonly not supported by this bus type Testing: -drive file=TEST_DIR/t.qcow2,if=virtio,readonly=on -qququiquit +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) qququiquit Testing: -drive file=TEST_DIR/t.qcow2,if=scsi,readonly=on -qququiquit +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) qququiquit Testing: -drive file=TEST_DIR/t.qcow2,if=none,id=disk,readonly=on -device ide-cd,drive=disk -qququiquit +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) qququiquit Testing: -drive file=TEST_DIR/t.qcow2,if=none,id=disk,readonly=on -device lsi53c895a -device scsi-cd,drive=disk -qququiquit +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) qququiquit Testing: -drive file=TEST_DIR/t.qcow2,if=none,id=disk,readonly=on -device ide-drive,drive=disk -QEMU_PROG: -device ide-drive,drive=disk: Can't use a read-only drive +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) QEMU_PROG: -device ide-drive,drive=disk: Can't use a read-only drive +QEMU_PROG: -device ide-drive,drive=disk: Device initialization failed. QEMU_PROG: -device ide-drive,drive=disk: Device 'ide-drive' could not be initialized Testing: -drive file=TEST_DIR/t.qcow2,if=none,id=disk,readonly=on -device ide-hd,drive=disk -QEMU_PROG: -device ide-hd,drive=disk: Can't use a read-only drive +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) QEMU_PROG: -device ide-hd,drive=disk: Can't use a read-only drive +QEMU_PROG: -device ide-hd,drive=disk: Device initialization failed. QEMU_PROG: -device ide-hd,drive=disk: Device 'ide-hd' could not be initialized Testing: -drive file=TEST_DIR/t.qcow2,if=none,id=disk,readonly=on -device lsi53c895a -device scsi-disk,drive=disk -qququiquit +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) qququiquit Testing: -drive file=TEST_DIR/t.qcow2,if=none,id=disk,readonly=on -device lsi53c895a -device scsi-hd,drive=disk -qququiquit +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) qququiquit === Cache modes === Testing: -drive media=cdrom,cache=none -qququiquit +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) qququiquit Testing: -drive media=cdrom,cache=directsync -qququiquit +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) qququiquit Testing: -drive media=cdrom,cache=writeback -qququiquit +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) qququiquit Testing: -drive media=cdrom,cache=writethrough -qququiquit +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) qququiquit Testing: -drive media=cdrom,cache=unsafe -qququiquit +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) qququiquit Testing: -drive media=cdrom,cache=invalid_value QEMU_PROG: -drive media=cdrom,cache=invalid_value: invalid cache option @@ -163,7 +205,8 @@ QEMU_PROG: -drive media=cdrom,cache=invalid_value: invalid cache option === Specifying the protocol layer === Testing: -drive file=TEST_DIR/t.qcow2,file.driver=file -qququiquit +QEMU 1.5.50 monitor - type 'help' for more information +(qemu) qququiquit Testing: -drive file=TEST_DIR/t.qcow2,file.driver=qcow2 QEMU_PROG: -drive file=TEST_DIR/t.qcow2,file.driver=qcow2: Can't use 'qcow2' as a block driver for the protocol level diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter index dcf6391ea2..9dbcae8d8c 100644 --- a/tests/qemu-iotests/common.filter +++ b/tests/qemu-iotests/common.filter @@ -155,7 +155,7 @@ _filter_qemu_io() # replace occurrences of QEMU_PROG with "qemu" _filter_qemu() { - sed -e "s#^$(basename $QEMU_PROG):#QEMU_PROG:#" + sed -e "s#\\(^\\|(qemu) \\)$(basename $QEMU_PROG):#\1QEMU_PROG:#" } # make sure this script returns success From f0f0fdfeec6c67ad374114ecc4b3e3ccde5e94d2 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 5 Jul 2013 13:48:01 +0200 Subject: [PATCH 5/8] block: Add return value for bdrv_flush_all() bdrv_flush() can fail, and bdrv_flush_all() should return an error as well if this happens for a block device. It returns the first error return now, but still at least tries to flush the remaining devices even in error cases. Signed-off-by: Kevin Wolf Reviewed-by: Stefan Hajnoczi --- block.c | 10 ++++++++-- include/block/block.h | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/block.c b/block.c index 2df65c88bf..b56024113b 100644 --- a/block.c +++ b/block.c @@ -2910,13 +2910,19 @@ int bdrv_get_flags(BlockDriverState *bs) return bs->open_flags; } -void bdrv_flush_all(void) +int bdrv_flush_all(void) { BlockDriverState *bs; + int result = 0; QTAILQ_FOREACH(bs, &bdrv_states, list) { - bdrv_flush(bs); + int ret = bdrv_flush(bs); + if (ret < 0 && !result) { + result = ret; + } } + + return result; } int bdrv_has_zero_init_1(BlockDriverState *bs) diff --git a/include/block/block.h b/include/block/block.h index eeb48162af..b6b9014a9c 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -267,7 +267,7 @@ void bdrv_clear_incoming_migration_all(void); /* Ensure contents are flushed to disk. */ int bdrv_flush(BlockDriverState *bs); int coroutine_fn bdrv_co_flush(BlockDriverState *bs); -void bdrv_flush_all(void); +int bdrv_flush_all(void); void bdrv_close_all(void); void bdrv_drain_all(void); From 5698346391b306c2c84358c68ee897c095d714cc Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 5 Jul 2013 13:49:54 +0200 Subject: [PATCH 6/8] cpus: Add return value for vm_stop() If flushing the block devices fails, return an error. The VM is stopped anyway. Signed-off-by: Kevin Wolf Reviewed-by: Stefan Hajnoczi --- cpus.c | 20 +++++++++++++------- include/sysemu/sysemu.h | 4 ++-- stubs/vm-stop.c | 2 +- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/cpus.c b/cpus.c index f141428225..29277e109f 100644 --- a/cpus.c +++ b/cpus.c @@ -434,17 +434,21 @@ bool cpu_is_stopped(CPUState *cpu) return !runstate_is_running() || cpu->stopped; } -static void do_vm_stop(RunState state) +static int do_vm_stop(RunState state) { + int ret = 0; + if (runstate_is_running()) { cpu_disable_ticks(); pause_all_vcpus(); runstate_set(state); vm_state_notify(0, state); bdrv_drain_all(); - bdrv_flush_all(); + ret = bdrv_flush_all(); monitor_protocol_event(QEVENT_STOP, NULL); } + + return ret; } static bool cpu_can_run(CPUState *cpu) @@ -1070,7 +1074,7 @@ void cpu_stop_current(void) } } -void vm_stop(RunState state) +int vm_stop(RunState state) { if (qemu_in_vcpu_thread()) { qemu_system_vmstop_request(state); @@ -1079,19 +1083,21 @@ void vm_stop(RunState state) * vm_stop() has been requested. */ cpu_stop_current(); - return; + return 0; } - do_vm_stop(state); + + return do_vm_stop(state); } /* does a state transition even if the VM is already stopped, current state is forgotten forever */ -void vm_stop_force_state(RunState state) +int vm_stop_force_state(RunState state) { if (runstate_is_running()) { - vm_stop(state); + return vm_stop(state); } else { runstate_set(state); + return 0; } } diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index d85bdc0cac..3caeb66eb2 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -35,8 +35,8 @@ void vm_state_notify(int running, RunState state); #define VMRESET_REPORT true void vm_start(void); -void vm_stop(RunState state); -void vm_stop_force_state(RunState state); +int vm_stop(RunState state); +int vm_stop_force_state(RunState state); typedef enum WakeupReason { QEMU_WAKEUP_REASON_OTHER = 0, diff --git a/stubs/vm-stop.c b/stubs/vm-stop.c index 45689354f6..f82c897dfe 100644 --- a/stubs/vm-stop.c +++ b/stubs/vm-stop.c @@ -1,7 +1,7 @@ #include "qemu-common.h" #include "sysemu/sysemu.h" -void vm_stop(RunState state) +int vm_stop(RunState state) { abort(); } From 0e1146a7a011a69d8cbc958b4f7ebad186730fc3 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 5 Jul 2013 13:54:55 +0200 Subject: [PATCH 7/8] migration: Fail migration on bdrv_flush_all() error If bdrv_flush_all() returns an error, there is an inconsistency in the view of an image file between the source and the destination host. Completing the migration would lead to corruption. Better abort migration in this case. To reproduce this case, try the following (ensures that there is something to flush, and then fails that flush): $ qemu-img create -f qcow2 test.qcow2 1G $ cat blkdebug.cfg [inject-error] event = "flush_to_os" errno = "5" $ qemu-system-x86_64 -hda blkdebug:blkdebug.cfg:test.qcow2 -monitor stdio (qemu) qemu-io ide0-hd0 "write 0 4k" (qemu) migrate ... Signed-off-by: Kevin Wolf Reviewed-by: Stefan Hajnoczi --- migration.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/migration.c b/migration.c index 635a7e7a08..0681d8e5f1 100644 --- a/migration.c +++ b/migration.c @@ -527,15 +527,26 @@ static void *migration_thread(void *opaque) if (pending_size && pending_size >= max_size) { qemu_savevm_state_iterate(s->file); } else { + int ret; + DPRINTF("done iterating\n"); qemu_mutex_lock_iothread(); start_time = qemu_get_clock_ms(rt_clock); qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER); old_vm_running = runstate_is_running(); - vm_stop_force_state(RUN_STATE_FINISH_MIGRATE); - qemu_file_set_rate_limit(s->file, INT_MAX); - qemu_savevm_state_complete(s->file); + + ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE); + if (ret >= 0) { + qemu_file_set_rate_limit(s->file, INT_MAX); + qemu_savevm_state_complete(s->file); + } qemu_mutex_unlock_iothread(); + + if (ret < 0) { + migrate_finish_set_state(s, MIG_STATE_ERROR); + break; + } + if (!qemu_file_get_error(s->file)) { migrate_finish_set_state(s, MIG_STATE_COMPLETED); break; From a62eaa26c1d6d48fbdc3ac1d32bd1314f5fdc8c9 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Mon, 15 Jul 2013 11:25:55 +0200 Subject: [PATCH 8/8] ahci: Fix FLUSH command AHCI couldn't cope with asynchronous commands that aren't doing DMA, it simply wouldn't complete them. Due to the bug fixed in commit f68ec837, FLUSH commands would seem to have completed immediately even if they were still running on the host. After the commit, they would simply hang and never unset the BSY bit, rendering AHCI unusable on any OS sending flushes. This patch adds another callback for the completion of asynchronous commands. This is what AHCI really wants to use for its command completion logic rather than an DMA completion callback. Cc: qemu-stable@nongnu.org Signed-off-by: Kevin Wolf Reviewed-by: Stefan Hajnoczi --- hw/ide/ahci.c | 8 +++++++- hw/ide/core.c | 9 +++++++++ hw/ide/internal.h | 1 + 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 97eddec920..1d863b5784 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1106,10 +1106,15 @@ static int ahci_dma_add_status(IDEDMA *dma, int status) } static int ahci_dma_set_inactive(IDEDMA *dma) +{ + return 0; +} + +static int ahci_async_cmd_done(IDEDMA *dma) { AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma); - DPRINTF(ad->port_no, "dma done\n"); + DPRINTF(ad->port_no, "async cmd done\n"); /* update d2h status */ ahci_write_fis_d2h(ad, NULL); @@ -1144,6 +1149,7 @@ static const IDEDMAOps ahci_dma_ops = { .set_unit = ahci_dma_set_unit, .add_status = ahci_dma_add_status, .set_inactive = ahci_dma_set_inactive, + .async_cmd_done = ahci_async_cmd_done, .restart_cb = ahci_dma_restart_cb, .reset = ahci_dma_reset, }; diff --git a/hw/ide/core.c b/hw/ide/core.c index 03d1cfa7d2..a73af7252a 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -568,10 +568,18 @@ static void dma_buf_commit(IDEState *s) qemu_sglist_destroy(&s->sg); } +static void ide_async_cmd_done(IDEState *s) +{ + if (s->bus->dma->ops->async_cmd_done) { + s->bus->dma->ops->async_cmd_done(s->bus->dma); + } +} + void ide_set_inactive(IDEState *s) { s->bus->dma->aiocb = NULL; s->bus->dma->ops->set_inactive(s->bus->dma); + ide_async_cmd_done(s); } void ide_dma_error(IDEState *s) @@ -804,6 +812,7 @@ static void ide_flush_cb(void *opaque, int ret) bdrv_acct_done(s->bs, &s->acct); s->status = READY_STAT | SEEK_STAT; + ide_async_cmd_done(s); ide_set_irq(s->bus); } diff --git a/hw/ide/internal.h b/hw/ide/internal.h index 03f14896d6..048a052143 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -433,6 +433,7 @@ struct IDEDMAOps { DMAIntFunc *set_unit; DMAIntFunc *add_status; DMAFunc *set_inactive; + DMAFunc *async_cmd_done; DMARestartFunc *restart_cb; DMAFunc *reset; };