On 10/27/2014 12:58 AM, Luyao Huang wrote:
When pass None to time, it will set guest time to 0. When pass an empty dictionary, it will report error. Allow a one-element dictionary which contains 'seconds' or 'nseconds', setting JUST seconds will do the sane thing of passing 0 for nseconds, instead of erroring out.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- libvirt-override-virDomain.py | 2 ++ libvirt-override.c | 26 +++++++++++++++++++++++--- 2 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/libvirt-override-virDomain.py b/libvirt-override-virDomain.py index a50ec0d..d788657 100644 --- a/libvirt-override-virDomain.py +++ b/libvirt-override-virDomain.py @@ -69,6 +69,8 @@ def setTime(self, time=None, flags=0): """Set guest time to the given value. @time is a dict conatining
s/conatining/containing/ while you are at it.
'seconds' field for seconds and 'nseconds' field for nanosecons """
and s/nanosecons/nanoseconds/
+ if time is None: + time = {'nseconds': 0, 'seconds': 0L}
I'd rather that we error out for None instead of silently converting to 0. Using all 0 (aka 1970) is usually the wrong thing. Likewise, while defaulting nseconds to 0 makes sense, I think we should require seconds to be present.
ret = libvirtmod.virDomainSetTime(self._o, time, flags) if ret == -1: raise libvirtError ('virDomainSetTime() failed', dom=self) return ret diff --git a/libvirt-override.c b/libvirt-override.c index a53b46f..5c016f9 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -7798,8 +7798,28 @@ libvirt_virDomainSetTime(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
py_dict_size = PyDict_Size(py_dict); + + if (py_dict_size == 1) { + PyObject *pyobj_seconds, *pyobj_nseconds; + + if ((pyobj_seconds = PyDict_GetItemString(py_dict, "seconds")) && + (libvirt_longlongUnwrap(pyobj_seconds, &seconds) < 0)) { + PyErr_Format(PyExc_LookupError, "malformed 'seconds'"); + goto cleanup; + }
- if (py_dict_size == 2) { + if ((pyobj_nseconds = PyDict_GetItemString(py_dict, "nseconds")) && + (libvirt_uintUnwrap(pyobj_nseconds, &nseconds) < 0)) { + PyErr_Format(PyExc_LookupError, "malformed 'nseconds'"); + goto cleanup; + } + + if (!pyobj_nseconds && !pyobj_seconds) { + PyErr_Format(PyExc_LookupError, "Dictionary must contain " + "'seconds' or 'nseconds'"); + goto cleanup; + } + } else if (py_dict_size == 2) { PyObject *pyobj_seconds, *pyobj_nseconds;
This feels overly complex. I think all we really need is: validate that py_dict is a dict (and not None, per my argument above) if dict contains seconds: populate seconds else: error out if dict contains nseconds: populate nseconds else: nseconds = 0 if dict contains anything else: error out
if (!(pyobj_seconds = PyDict_GetItemString(py_dict, "seconds")) || @@ -7813,9 +7833,9 @@ libvirt_virDomainSetTime(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { PyErr_Format(PyExc_LookupError, "malformed or missing 'nseconds'"); goto cleanup; } - } else if (py_dict_size > 0) { + } else if (py_dict_size >= 0) { PyErr_Format(PyExc_LookupError, "Dictionary must contain " - "'seconds' and 'nseconds'"); + "'seconds' or 'nseconds'");
The error message needs touching up if nseconds is going to be optional. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org