On 03/28/2012 12:02 PM, Eric Blake wrote: > On 03/23/2012 04:08 AM, Christophe Fergeau wrote: >> libvirt documentation for channels with type 'spicevmc' says that the >> 'target' child node has: >> "an optional attribute name controls how the guest will have access >> to the channel, and defaults to name='com.redhat.spice.0'." >> >> However, this default value is never set in libvirt code base, >> there's only a check in qemu_command.c to error out if the name >> attribute doesn't have the expected value (if it's set). >> >> This commit sets a default target name for spicevmc channels during >> the domain configuration parsing so that the code agrees with the >> documentation. >> --- >> src/conf/domain_conf.c | 7 +++++++ >> 1 files changed, 7 insertions(+), 0 deletions(-) > ACK, and okay to push before 0.9.11. > >> } else { >> def->source.data.spicevmc = VIR_DOMAIN_CHR_SPICEVMC_VDAGENT; >> + if (!def->target.name) { >> + def->target.name = strdup("com.redhat.spice.0"); >> + if (!def->target.name) { >> + virReportOOMError(); >> + goto error; >> + } >> + } >> } >> } >>
I know I'm a bit late to the party (I missed your mail before), but would it have been possible to do the "default" processing in the caller rather than in the parsing function itself? Setting it here means that if the default changes, previously defined guests will be stuck with the old default (since it will be encoded into re-formatted XML saved to disk), and assigning default values in parsing functions has in the past led to trouble (e.g. the fact that virDomainNetDefParseXML adds a mac address when one isn't specified caused trouble because parsing the same XML twice gave difference results; that particular problem wouldn't happen here, obviously). -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list