Ala Hino has uploaded a new change for review.
Change subject: qemuimg: Introduce InvalidOutput exception ......................................................................
qemuimg: Introduce InvalidOutput exception
Raised when the command output is not valid.
Change-Id: If3f801f05f130c2417e4946877e31030260269a1 Signed-off-by: Ala Hino ahino@redhat.com --- M lib/vdsm/qemuimg.py 1 file changed, 20 insertions(+), 5 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/08/65208/1
diff --git a/lib/vdsm/qemuimg.py b/lib/vdsm/qemuimg.py index 28cddee..8f6dc35 100644 --- a/lib/vdsm/qemuimg.py +++ b/lib/vdsm/qemuimg.py @@ -65,6 +65,21 @@ self.cmd, self.ecode, self.stdout, self.stderr, self.message)
+class InvalidOutput(QImgError): + """ + Raised when the command output is not valid. + """ + + def __init__(self, cmd, stdout, message): + self.cmd = cmd + self.stdout = stdout + self.message = message + + def __str__(self): + return "cmd=%s, stdout=%s, message=%s" % ( + self.cmd, self.stdout, self.message) + + def info(image, format=None): cmd = [_qemuimg.cmd, "info", "--output", "json"]
@@ -79,7 +94,7 @@ try: qemu_info = _parse_qemuimg_json(out) except ValueError: - raise QImgError(cmd, rc, out, err, "Failed to process qemu-img output") + raise InvalidOutput(cmd, out, "Failed to process qemu-img output")
try: info = { @@ -87,7 +102,7 @@ 'virtualsize': qemu_info['virtual-size'], } except KeyError as key: - raise QImgError(cmd, rc, out, err, "Missing field: %r" % key) + raise InvalidOutput(cmd, out, "Missing field: %r" % key)
if 'cluster-size' in qemu_info: info['clustersize'] = qemu_info['cluster-size'] @@ -97,7 +112,7 @@ try: info['compat'] = qemu_info['format-specific']['data']['compat'] except KeyError: - raise QImgError(cmd, rc, out, err, "'compat' expected but not found") + raise InvalidOutput(cmd, out, "'compat' expected but not found")
return info
@@ -148,11 +163,11 @@ try: qemu_check = _parse_qemuimg_json(out) except ValueError: - raise QImgError(cmd, rc, out, err, "Failed to process qemu-img output") + raise InvalidOutput(cmd, out, "Failed to process qemu-img output") try: return {"offset": qemu_check["image-end-offset"]} except KeyError: - raise QImgError(cmd, rc, out, err, "unable to parse qemu-img check output") + raise InvalidOutput(cmd, out, "unable to parse qemu-img check output")
def convert(srcImage, dstImage, srcFormat=None, dstFormat=None,
gerrit-hooks has posted comments on this change.
Change subject: qemuimg: Introduce InvalidOutput exception ......................................................................
Patch Set 1:
* Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
Nir Soffer has posted comments on this change.
Change subject: qemuimg: Introduce InvalidOutput exception ......................................................................
Patch Set 1:
(1 comment)
Nice
https://gerrit.ovirt.org/#/c/65208/1//COMMIT_MSG Commit Message:
Line 5: CommitDate: 2016-10-06 21:31:05 +0300 Line 6: Line 7: qemuimg: Introduce InvalidOutput exception Line 8: Line 9: Raised when the command output is not valid. Lets give some more context:
Raised when qemu-img command succeeded, but its output cannot be parsed or missing required properties.
In this case we know that rc is 0, and stderr is probably empty, so including it in the error is not very useful. Line 10: Line 11: Change-Id: If3f801f05f130c2417e4946877e31030260269a1
Nir Soffer has posted comments on this change.
Change subject: qemuimg: Introduce InvalidOutput exception ......................................................................
Patch Set 1: Code-Review+1
gerrit-hooks has posted comments on this change.
Change subject: qemuimg: Introduce InvalidOutput exception ......................................................................
Patch Set 2:
* Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
Ala Hino has posted comments on this change.
Change subject: qemuimg: Introduce InvalidOutput exception ......................................................................
Patch Set 1:
(1 comment)
https://gerrit.ovirt.org/#/c/65208/1//COMMIT_MSG Commit Message:
Line 5: CommitDate: 2016-10-06 21:31:05 +0300 Line 6: Line 7: qemuimg: Introduce InvalidOutput exception Line 8: Line 9: Raised when the command output is not valid.
Lets give some more context:
Done Line 10: Line 11: Change-Id: If3f801f05f130c2417e4946877e31030260269a1
Nir Soffer has posted comments on this change.
Change subject: qemuimg: Introduce InvalidOutput exception ......................................................................
Patch Set 1:
(1 comment)
https://gerrit.ovirt.org/#/c/65208/1/lib/vdsm/qemuimg.py File lib/vdsm/qemuimg.py:
Line 67: Line 68: class InvalidOutput(QImgError): Line 69: """ Line 70: Raised when the command output is not valid. Line 71: """ We can do this ugly hack to make sure that no client fail to get ecode and stderr attributes from this subclass:
ecode = 0 stderr = ""
A better design would probably be to inherit from qemuimg.Error, and have two different sub classes, inheriting nothing from the base class.
class Error(Exception): """ Base class for qemuimg errors """
CommandFailed(Error): def __init__(self, msg, cmd, rc, out, err): ...
InvalidOutput(Error): def __init__(self, msg, cmd, out): ...
This require changing callers to check for qemuimg.Error. Line 72: Line 73: def __init__(self, cmd, stdout, message): Line 74: self.cmd = cmd Line 75: self.stdout = stdout
gerrit-hooks has posted comments on this change.
Change subject: qemuimg: Introduce InvalidOutput exception ......................................................................
Patch Set 3:
* Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
Ala Hino has posted comments on this change.
Change subject: qemuimg: Introduce InvalidOutput exception ......................................................................
Patch Set 2:
(1 comment)
https://gerrit.ovirt.org/#/c/65208/1/lib/vdsm/qemuimg.py File lib/vdsm/qemuimg.py:
Line 67: Line 68: class InvalidOutput(QImgError): Line 69: """ Line 70: Raised when the command output is not valid. Line 71: """
We can do this ugly hack to make sure that no client fail to get ecode and
Done Line 72: Line 73: def __init__(self, cmd, stdout, message): Line 74: self.cmd = cmd Line 75: self.stdout = stdout
Nir Soffer has posted comments on this change.
Change subject: qemuimg: Introduce InvalidOutput exception ......................................................................
Patch Set 3:
(3 comments)
https://gerrit.ovirt.org/#/c/65208/3/lib/vdsm/qemuimg.py File lib/vdsm/qemuimg.py:
Line 67: Line 68: class InvalidOutput(QImgError): Line 69: """ Line 70: Raised when the command output is not valid. Line 71: """ You wan to use class attributes (like java static member):
ecode = 0 stderr = ""
Unlike java, you can access these like an instance attribute:
e = InvalidOutput([], "", "") e.ecode -> 0 e.stderr -> "" Line 72: Line 73: def __init__(self, cmd, stdout, message): Line 74: self.cmd = cmd Line 75: self.ecode = 0
Line 71: """ Line 72: Line 73: def __init__(self, cmd, stdout, message): Line 74: self.cmd = cmd Line 75: self.ecode = 0 This creates an instance variable with constant value for each instance. Line 76: self.stdout = stdout Line 77: self.stderr = "" Line 78: self.message = message Line 79:
Line 73: def __init__(self, cmd, stdout, message): Line 74: self.cmd = cmd Line 75: self.ecode = 0 Line 76: self.stdout = stdout Line 77: self.stderr = "" Same Line 78: self.message = message Line 79: Line 80: def __str__(self): Line 81: return "cmd=%s, stdout=%s, message=%s" % (
gerrit-hooks has posted comments on this change.
Change subject: qemuimg: Introduce InvalidOutput exception ......................................................................
Patch Set 4:
* Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
Ala Hino has posted comments on this change.
Change subject: qemuimg: Introduce InvalidOutput exception ......................................................................
Patch Set 3:
(3 comments)
https://gerrit.ovirt.org/#/c/65208/3/lib/vdsm/qemuimg.py File lib/vdsm/qemuimg.py:
Line 67: Line 68: class InvalidOutput(QImgError): Line 69: """ Line 70: Raised when the command output is not valid. Line 71: """
You wan to use class attributes (like java static member):
Done Line 72: Line 73: def __init__(self, cmd, stdout, message): Line 74: self.cmd = cmd Line 75: self.ecode = 0
Line 71: """ Line 72: Line 73: def __init__(self, cmd, stdout, message): Line 74: self.cmd = cmd Line 75: self.ecode = 0
This creates an instance variable with constant value for each instance.
Done Line 76: self.stdout = stdout Line 77: self.stderr = "" Line 78: self.message = message Line 79:
Line 73: def __init__(self, cmd, stdout, message): Line 74: self.cmd = cmd Line 75: self.ecode = 0 Line 76: self.stdout = stdout Line 77: self.stderr = ""
Same
Done Line 78: self.message = message Line 79: Line 80: def __str__(self): Line 81: return "cmd=%s, stdout=%s, message=%s" % (
Nir Soffer has posted comments on this change.
Change subject: qemuimg: Introduce InvalidOutput exception ......................................................................
Patch Set 4: Continuous-Integration+1
Please fix pep8 errors in this topic.
gerrit-hooks has posted comments on this change.
Change subject: qemuimg: Introduce InvalidOutput exception ......................................................................
Patch Set 5:
* Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
Nir Soffer has posted comments on this change.
Change subject: qemuimg: Introduce InvalidOutput exception ......................................................................
Patch Set 5: Code-Review+1
gerrit-hooks has posted comments on this change.
Change subject: qemuimg: Introduce InvalidOutput exception ......................................................................
Patch Set 6:
* Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
Ala Hino has posted comments on this change.
Change subject: qemuimg: Introduce InvalidOutput exception ......................................................................
Patch Set 6: Verified+1
Nir Soffer has posted comments on this change.
Change subject: qemuimg: Introduce InvalidOutput exception ......................................................................
Patch Set 6:
Waiting for Adam review.
Nir Soffer has posted comments on this change.
Change subject: qemuimg: Introduce InvalidOutput exception ......................................................................
Patch Set 6: Code-Review+2
Nir Soffer has posted comments on this change.
Change subject: qemuimg: Introduce InvalidOutput exception ......................................................................
Patch Set 6:
Lets move faster, If Adam has issues with this we will enhance this later.
Nir Soffer has submitted this change and it was merged.
Change subject: qemuimg: Introduce InvalidOutput exception ......................................................................
qemuimg: Introduce InvalidOutput exception
Raised when qemu-img command succeeded, but its output cannot be parsed or missing required properties. In this case we know that rc is 0, and stderr is probably empty, so including it in the error is not very useful.
Change-Id: If3f801f05f130c2417e4946877e31030260269a1 Signed-off-by: Ala Hino ahino@redhat.com Reviewed-on: https://gerrit.ovirt.org/65208 Continuous-Integration: Jenkins CI Reviewed-by: Nir Soffer nsoffer@redhat.com --- M lib/vdsm/qemuimg.py 1 file changed, 23 insertions(+), 7 deletions(-)
Approvals: Nir Soffer: Looks good to me, approved Jenkins CI: Passed CI tests Ala Hino: Verified
gerrit-hooks has posted comments on this change.
Change subject: qemuimg: Introduce InvalidOutput exception ......................................................................
Patch Set 7:
* Update tracker: IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found.
vdsm-patches@lists.fedorahosted.org