Commit 7676e2c5 authored by Jeff Cody's avatar Jeff Cody Committed by Stefan Hajnoczi
Browse files

block: make 'top' argument to block-commit optional



Now that active layer block-commit is supported, the 'top' argument
no longer needs to be mandatory.

Change it to optional, with the default being the active layer in the
device chain.

[kwolf: Rebased and resolved conflict in tests/qemu-iotests/040]

Reviewed-by: default avatarEric Blake <eblake@redhat.com>
Reviewed-by: default avatarBenoit Canet <benoit@irqsave.net>
Signed-off-by: default avatarJeff Cody <jcody@redhat.com>
Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
Signed-off-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
parent c891e3bb
Loading
Loading
Loading
Loading
+14 −2
Original line number Diff line number Diff line
@@ -1913,7 +1913,8 @@ void qmp_block_stream(const char *device, bool has_base,
}

void qmp_block_commit(const char *device,
                      bool has_base, const char *base, const char *top,
                      bool has_base, const char *base,
                      bool has_top, const char *top,
                      bool has_speed, int64_t speed,
                      Error **errp)
{
@@ -1932,6 +1933,11 @@ void qmp_block_commit(const char *device,
    /* drain all i/o before commits */
    bdrv_drain_all();

    /* Important Note:
     *  libvirt relies on the DeviceNotFound error class in order to probe for
     *  live commit feature versions; for this to work, we must make sure to
     *  perform the device lookup before any generic errors that may occur in a
     *  scenario in which all optional arguments are omitted. */
    bs = bdrv_find(device);
    if (!bs) {
        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
@@ -1945,7 +1951,7 @@ void qmp_block_commit(const char *device,
    /* default top_bs is the active layer */
    top_bs = bs;

    if (top) {
    if (has_top && top) {
        if (strcmp(bs->filename, top) != 0) {
            top_bs = bdrv_find_backing_image(bs, top);
        }
@@ -1967,6 +1973,12 @@ void qmp_block_commit(const char *device,
        return;
    }

    /* Do not allow attempts to commit an image into itself */
    if (top_bs == base_bs) {
        error_setg(errp, "cannot commit an image into itself");
        return;
    }

    if (top_bs == bs) {
        commit_active_start(bs, base_bs, speed, on_error, block_job_cb,
                            bs, &local_err);
+4 −3
Original line number Diff line number Diff line
@@ -690,8 +690,9 @@
# @base:   #optional The file name of the backing image to write data into.
#                    If not specified, this is the deepest backing image
#
# @top:              The file name of the backing image within the image chain,
#                    which contains the topmost data to be committed down.
# @top:    #optional The file name of the backing image within the image chain,
#                    which contains the topmost data to be committed down. If
#                    not specified, this is the active layer.
#
#                    If top == base, that is an error.
#                    If top == active, the job will not be completed by itself,
@@ -719,7 +720,7 @@
#
##
{ 'command': 'block-commit',
  'data': { 'device': 'str', '*base': 'str', 'top': 'str',
  'data': { 'device': 'str', '*base': 'str', '*top': 'str',
            '*speed': 'int' } }

##
+3 −2
Original line number Diff line number Diff line
@@ -985,7 +985,7 @@ EQMP

    {
        .name       = "block-commit",
        .args_type  = "device:B,base:s?,top:s,speed:o?",
        .args_type  = "device:B,base:s?,top:s?,speed:o?",
        .mhandler.cmd_new = qmp_marshal_input_block_commit,
    },

@@ -1003,7 +1003,8 @@ Arguments:
          If not specified, this is the deepest backing image
          (json-string, optional)
- "top":  The file name of the backing image within the image chain,
          which contains the topmost data to be committed down.
          which contains the topmost data to be committed down. If
          not specified, this is the active layer. (json-string, optional)

          If top == base, that is an error.
          If top == active, the job will not be completed by itself,
+18 −10
Original line number Diff line number Diff line
@@ -35,11 +35,7 @@ test_img = os.path.join(iotests.test_dir, 'test.img')
class ImageCommitTestCase(iotests.QMPTestCase):
    '''Abstract base class for image commit test cases'''

    def run_commit_test(self, top, base, need_ready=False):
        self.assert_no_active_block_jobs()
        result = self.vm.qmp('block-commit', device='drive0', top=top, base=base)
        self.assert_qmp(result, 'return', {})

    def wait_for_complete(self, need_ready=False):
        completed = False
        ready = False
        while not completed:
@@ -62,6 +58,18 @@ class ImageCommitTestCase(iotests.QMPTestCase):
        self.assert_no_active_block_jobs()
        self.vm.shutdown()

    def run_commit_test(self, top, base, need_ready=False):
        self.assert_no_active_block_jobs()
        result = self.vm.qmp('block-commit', device='drive0', top=top, base=base)
        self.assert_qmp(result, 'return', {})
        self.wait_for_complete(need_ready)

    def run_default_commit_test(self):
        self.assert_no_active_block_jobs()
        result = self.vm.qmp('block-commit', device='drive0')
        self.assert_qmp(result, 'return', {})
        self.wait_for_complete()

class TestSingleDrive(ImageCommitTestCase):
    image_len = 1 * 1024 * 1024
    test_len = 1 * 1024 * 256
@@ -113,17 +121,17 @@ class TestSingleDrive(ImageCommitTestCase):
        self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', backing_img).find("verification failed"))
        self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed"))

    def test_top_is_default_active(self):
        self.run_default_commit_test()
        self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', backing_img).find("verification failed"))
        self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed"))

    def test_top_and_base_reversed(self):
        self.assert_no_active_block_jobs()
        result = self.vm.qmp('block-commit', device='drive0', top='%s' % backing_img, base='%s' % mid_img)
        self.assert_qmp(result, 'error/class', 'GenericError')
        self.assert_qmp(result, 'error/desc', 'Base \'%s\' not found' % mid_img)

    def test_top_omitted(self):
        self.assert_no_active_block_jobs()
        result = self.vm.qmp('block-commit', device='drive0')
        self.assert_qmp(result, 'error/class', 'GenericError')
        self.assert_qmp(result, 'error/desc', "Parameter 'top' is missing")

class TestRelativePaths(ImageCommitTestCase):
    image_len = 1 * 1024 * 1024