On 12/26/2015 12:18 PM, Dmitry Mishin wrote:
Signed-off-by: Dmitry Mishin <dim@virtuozzo.com> --- src/conf/domain_conf.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f210c0b..8895e10 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1723,10 +1723,11 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest,
switch (src->type) { case VIR_DOMAIN_CHR_TYPE_FILE: - dest->data.file.append = src->data.file.append; case VIR_DOMAIN_CHR_TYPE_PTY: case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_PIPE: + if (src->type == VIR_DOMAIN_CHR_TYPE_FILE) + dest->data.file.append = src->data.file.append; if (VIR_STRDUP(dest->data.file.path, src->data.file.path) < 0) return -1; break; @@ -9386,12 +9387,12 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
switch ((virDomainChrType) def->type) { case VIR_DOMAIN_CHR_TYPE_FILE: - if (!append) - append = virXMLPropString(cur, "append"); case VIR_DOMAIN_CHR_TYPE_PTY: case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_PIPE: case VIR_DOMAIN_CHR_TYPE_UNIX: + if (!append && def->type == VIR_DOMAIN_CHR_TYPE_FILE) + append = virXMLPropString(cur, "append"); /* PTY path is only parsed from live xml. */ if (!path && (def->type != VIR_DOMAIN_CHR_TYPE_PTY || @@ -9476,15 +9477,15 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, break;
case VIR_DOMAIN_CHR_TYPE_FILE: + case VIR_DOMAIN_CHR_TYPE_PTY: + case VIR_DOMAIN_CHR_TYPE_DEV: + case VIR_DOMAIN_CHR_TYPE_PIPE: if (append &&
Wouldn't this last one be : if (append && def->type == VIR_DOMAIN_CHR_TYPE_FILE &&
(def->data.file.append = virTristateSwitchTypeFromString(append)) <= 0) {
NB: Other uses of def->data.file.append could have used the virTristateSwitch constants (VIR_TRISTATE_SWITCH_ABSENT, VIR_TRISTATE_SWITCH_ON, and VIR_TRISTATE_SWITCH_OFF). That is rather than "if (dev->data.file.append)", it could be "if (dev->data.file.append != VIR_TRISTATE_SWITCH_ABSENT). It's just "safer" that way. Does adding ",append=off" make much sense? Or do you expect some day in the future that append=on becomes the "default"? You could have added an example where append='off' - it shouldn't matter, but it is a legal option and the right XML should be generated and the right .args would have append=off added to the command line. John
virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid append attribute value '%s'"), append); goto error; } - case VIR_DOMAIN_CHR_TYPE_PTY: - case VIR_DOMAIN_CHR_TYPE_DEV: - case VIR_DOMAIN_CHR_TYPE_PIPE: if (!path && def->type != VIR_DOMAIN_CHR_TYPE_PTY) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s",