Re: [libvirt] [PATCH 08/10] Define XML syntax for password expiry

On 11/01/2010 12:17 PM, Daniel P. Berrange wrote: > This extends the XML syntax for <graphics> to allow a password > expiry time to be set > > eg > > <graphics type='vnc' port='5900' autoport='yes' keymap='en-us' passwd='12345' passwdValidTo='2010-04-09T15:51:00'/> > > The timestamp is in UTC. > > * src/conf/domain_conf.h: Pull passwd out into separate struct > virDomainGraphicsAuthDef to allow sharing between VNC & SPICE > * src/conf/domain_conf.c: Add parsing/formatting of new passwdValidTo > argument > * src/opennebula/one_conf.c, src/qemu/qemu_conf.c, src/qemu/qemu_driver.c, > src/xen/xend_internal.c, src/xen/xm_internal.c: Update for changed > struct containing VNC password > --- > src/conf/domain_conf.c | 102 +++++++++++++++++++++++++++++++++++++++----- > src/conf/domain_conf.h | 13 +++++- > src/esx/esx_vmx.c | 6 +- > src/opennebula/one_conf.c | 4 +- > src/qemu/qemu_conf.c | 4 +- > src/qemu/qemu_driver.c | 20 ++++---- > src/xen/xend_internal.c | 12 +++--- > src/xen/xm_internal.c | 12 +++--- > 8 files changed, 130 insertions(+), 43 deletions(-) Where's the changes to docs/schemas/domain.rng and docs/formatdomain.html.in? Is passwdValidTo any better off as seconds since the Epoch (date +%s) rather than an ISO time (date +%FT%T)? It boils down to a question of which format is easier for machines to handle. Or maybe we should support both formats, as it's pretty easy to tell them apart?

> + VIR_FREE(validTo); > + > + tm.tm_year -= 1900; /* Human epoch starts at 0 BC, not 1900BC */ > + tm.tm_mon--; /* Humans start months at 1, computers at 0 */ > + > + /* XXX this is broken it needs to be UTC not localtime */ > + def->validTo = timegm(&tm); Is that XXX comment still correct, or are we using UTC time by virtue of the timegm() call?

> > +static void > +virDomainGraphicsAuthDefFormatAttr(virBufferPtr buf, > + virDomainGraphicsAuthDefPtr def) > +{ > + if (!def->passwd) > + return; > + > + virBufferEscapeString(buf, " passwd='%s'", > + def->passwd); Should this depend on whether VIR_DOMAIN_XML_SECURE is in effect... > + if (def->expires) { > + char strbuf[100]; > + struct tm tmbuf, *tm; > + tm = gmtime_r(&def->validTo, &tmbuf); > + strftime(strbuf, sizeof(strbuf), "%Y-%m-%dT%H:%M:%S", tm); > + virBufferVSprintf(buf, " passwdValidTo='%s'", strbuf); > + } > +} > + > static int > virDomainGraphicsDefFormat(virBufferPtr buf, > virDomainGraphicsDefPtr def, > @@ -6355,10 +6437,8 @@ virDomainGraphicsDefFormat(virBufferPtr buf, > virBufferEscapeString(buf, " keymap='%s'", > def->data.vnc.keymap); > > - if (def->data.vnc.passwd && > - (flags & VIR_DOMAIN_XML_SECURE)) > - virBufferEscapeString(buf, " passwd='%s'", > - def->data.vnc.passwd); > + if (flags & VIR_DOMAIN_XML_SECURE) > + virDomainGraphicsAuthDefFormatAttr(buf, &def->data.vnc.auth); rather than here, since it makes sense to include password expiry in the XML even if the password itself is not included?