Federico Simoncelli has uploaded a new change for review.
Change subject: lib: add create function to the qemuImg module ......................................................................
lib: add create function to the qemuImg module
Change-Id: I69522cd6231e511d891815248fab9a15cbc987e5 Signed-off-by: Federico Simoncelli fsimonce@redhat.com --- M lib/vdsm/qemuImg.py 1 file changed, 23 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/35/18935/1
diff --git a/lib/vdsm/qemuImg.py b/lib/vdsm/qemuImg.py index a2a5682..e9a328c 100644 --- a/lib/vdsm/qemuImg.py +++ b/lib/vdsm/qemuImg.py @@ -88,6 +88,29 @@ return info
+def create(image, size=None, format=None, backing=None, backingFormat=None): + cmd = [_qemuimg.cmd, "create"] + + if format: + cmd.extend(("-f", format)) + + if backing: + cmd.extend(("-b", backing)) + + if backingFormat: + cmd.extend(("-F", backingFormat)) + + cmd.append(image) + + if size: + cmd.append(int(size)) + + rc, out, err = utils.execCmd(cmd) + + if rc != 0: + raise QImgError(rc, out, err) + + def check(image, format=None): cmd = [_qemuimg.cmd, "check"]
oVirt Jenkins CI Server has posted comments on this change.
Change subject: lib: add create function to the qemuImg module ......................................................................
Patch Set 1:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4297/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3400/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4216/ : SUCCESS
Dan Kenigsberg has posted comments on this change.
Change subject: lib: add create function to the qemuImg module ......................................................................
Patch Set 1: Code-Review-1
(1 comment)
.................................................... File lib/vdsm/qemuImg.py Line 87: Line 88: return info Line 89: Line 90: Line 91: def create(image, size=None, format=None, backing=None, backingFormat=None): I find the arg order a bit awkward - I'd keep info()'s order of image first, then format, then the rest. Line 92: cmd = [_qemuimg.cmd, "create"] Line 93: Line 94: if format: Line 95: cmd.extend(("-f", format))
Federico Simoncelli has posted comments on this change.
Change subject: lib: add create function to the qemuImg module ......................................................................
Patch Set 1:
(1 comment)
.................................................... File lib/vdsm/qemuImg.py Line 87: Line 88: return info Line 89: Line 90: Line 91: def create(image, size=None, format=None, backing=None, backingFormat=None): On the contrary, I don't think it's awkward, it's just how qemu-img works. The mandatory arguments are image and size:
qemuImg.create("myimage.img", 1024**3)
The format by default is "raw", so you can omit it, but if you want you can still specify it easily:
qemuImg.create("myimage.img", 1024**3, qemuImg.FORMAT.QCOW2)
Now you might say that you prefer always being explicit about the format, that's a good practice, agreed, but it has nothing to do with writing a wrapper where the goal is to expose the exact behavior of the command.
The only reason for size being optional is that it can be automatically detected when a backing file is specified:
qemuImg.create("myimage.img", ..., backing="mybacking.img")
Now, the reason for not checking the size/backing presence is that we don't want to re-write checks, we'll rely on qemu-img failing (triggering the QImgError exception). Line 92: cmd = [_qemuimg.cmd, "create"] Line 93: Line 94: if format: Line 95: cmd.extend(("-f", format))
oVirt Jenkins CI Server has posted comments on this change.
Change subject: lib: add create function to the qemuImg module ......................................................................
Patch Set 2:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4348/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3451/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4267/ : SUCCESS
Dan Kenigsberg has posted comments on this change.
Change subject: lib: add create function to the qemuImg module ......................................................................
Patch Set 2: Code-Review+2
Thanks for the convincing explanation.
Yeela Kaplan has posted comments on this change.
Change subject: lib: add create function to the qemuImg module ......................................................................
Patch Set 2: Code-Review+1
great work! I was just writing the same patch :)
Federico Simoncelli has posted comments on this change.
Change subject: lib: add create function to the qemuImg module ......................................................................
Patch Set 2: Verified+1
In my current use case it works (backup api). Feel free to merge if you need this in other patches too.
In the future I'll send a patch to clean up other places that should be using this function instead of a direct call to qemu-img.
Dan Kenigsberg has submitted this change and it was merged.
Change subject: lib: add create function to the qemuImg module ......................................................................
lib: add create function to the qemuImg module
Change-Id: I69522cd6231e511d891815248fab9a15cbc987e5 Signed-off-by: Federico Simoncelli fsimonce@redhat.com Reviewed-on: http://gerrit.ovirt.org/18935 Reviewed-by: Dan Kenigsberg danken@redhat.com Reviewed-by: Yeela Kaplan ykaplan@redhat.com --- M lib/vdsm/qemuImg.py 1 file changed, 23 insertions(+), 0 deletions(-)
Approvals: Yeela Kaplan: Looks good to me, but someone else must approve Federico Simoncelli: Verified Dan Kenigsberg: Looks good to me, approved
vdsm-patches@lists.fedorahosted.org