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 <lhu...@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
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
