Thanks your reply and i have a new idea about when time = None.

On 10/28/2014 01:24 AM, Eric Blake wrote:
> 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.
Allow None seems work well with VIR_DOMAIN_TIME_SYNC(future for ), when 
flags=libvirt.VIR_DOMAIN_TIME_SYNC,
time will be optional.How about set time={'seconds': 
int(time.time()),'nseconds': 0} when time = None and add some
desc in Doc.If use this way the setTime will change to:
setTime() =   virsh domtime --now
setTime({'seconds':1234567}) = virsh domtime 1234567
setTime(flags=libvirt.VIR_DOMAIN_TIME_SYNC) = virsh domtime --sync

>
>>           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
Cool!I will use this way to check the seconds and nseconds.
>
>>   
>>           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.
Yes,how about change the error to

"Dictionary must contain 'seconds'"

>