<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Thanks your reply and i have a new idea about when time = None.<br>
<br>
<div class="moz-cite-prefix">On 10/28/2014 01:24 AM, Eric Blake
wrote:<br>
</div>
<blockquote cite="mid:544E7FC6.7050202@redhat.com" type="cite">
<pre wrap="">On 10/27/2014 12:58 AM, Luyao Huang wrote:
</pre>
<blockquote type="cite">
<pre wrap="">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 <a class="moz-txt-link-rfc2396E" href="mailto:lhuang@redhat.com"><lhuang@redhat.com></a>
---
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
</pre>
</blockquote>
<pre wrap="">
s/conatining/containing/ while you are at it.
</pre>
<blockquote type="cite">
<pre wrap=""> 'seconds' field for seconds and 'nseconds' field for nanosecons """
</pre>
</blockquote>
<pre wrap="">
and s/nanosecons/nanoseconds/
</pre>
<blockquote type="cite">
<pre wrap="">+ if time is None:
+ time = {'nseconds': 0, 'seconds': 0L}
</pre>
</blockquote>
<pre wrap="">
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.</pre>
</blockquote>
<small>Allow None seems work well with VIR_DOMAIN_TIME_SYNC(future
for ), when flags=libvirt.VIR_DOMAIN_TIME_SYNC, <br>
time will be optional.</small><small>How about set
time={'seconds': int(time.time()),'nseconds': 0} when time = None
and add some<br>
desc in Doc.If use this way the setTime will change to:<br>
setTime() = virsh domtime --now<br>
setTime({'seconds':1234567}) = virsh domtime 1</small><small>234567<br>
setTime(flags=</small><small><small>libvirt.VIR_DOMAIN_TIME_SYNC</small>)
= virsh domtime --sync<br>
<br>
</small>
<blockquote cite="mid:544E7FC6.7050202@redhat.com" type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap=""> 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;
</pre>
</blockquote>
<pre wrap="">
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</pre>
</blockquote>
<small>Cool!I will use this way to check the seconds and nseconds.</small><br>
<blockquote cite="mid:544E7FC6.7050202@redhat.com" type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">
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'");
</pre>
</blockquote>
<pre wrap="">
The error message needs touching up if nseconds is going to be optional.</pre>
</blockquote>
<small>Yes,how about change the error to </small><br>
<pre wrap="">"Dictionary must contain 'seconds'"</pre>
<blockquote cite="mid:544E7FC6.7050202@redhat.com" type="cite">
<pre wrap="">
</pre>
</blockquote>
<br>
</body>
</html>