[libvirt] [PATCH 2/3] conf: Add mode 'append' for chardevs with file source

[Daniel P. Berrange]

On Wed, Dec 23, 2015 at 04:01:24PM +0300, Dmitry Mishin wrote:
> Currently, there is no possibility for user to specify desired behaviour of
> output to file - truncate or append. This patch adds an ability to explicitly
> specify that user wants to preserve file's content on reopen.
> 
> Signed-off-by: Dmitry Mishin <dim at virtuozzo.com>
> ---
>  src/conf/domain_conf.c | 18 ++++++++++++++----
>  src/conf/domain_conf.h |  1 +
>  2 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 5200c27..ea854a7 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1722,9 +1722,10 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest,
>      virDomainChrSourceDefClear(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_FILE:
>      case VIR_DOMAIN_CHR_TYPE_PIPE:
>          if (VIR_STRDUP(dest->data.file.path, src->data.file.path) < 0)
>              return -1;
> @@ -1796,9 +1797,12 @@ virDomainChrSourceDefIsEqual(const virDomainChrSourceDef *src,
>          return false;
>  
>      switch ((virDomainChrType)src->type) {
> +    case VIR_DOMAIN_CHR_TYPE_FILE:
> +        return src->data.file.append == tgt->data.file.append &&
> +            STREQ_NULLABLE(src->data.file.path, tgt->data.file.path);
> +        break;
>      case VIR_DOMAIN_CHR_TYPE_PTY:
>      case VIR_DOMAIN_CHR_TYPE_DEV:
> -    case VIR_DOMAIN_CHR_TYPE_FILE:
>      case VIR_DOMAIN_CHR_TYPE_PIPE:
>          return STREQ_NULLABLE(src->data.file.path, tgt->data.file.path);
>          break;
> @@ -9468,9 +9472,10 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
>      case VIR_DOMAIN_CHR_TYPE_LAST:
>          break;
>  
> +    case VIR_DOMAIN_CHR_TYPE_FILE:
> +        def->data.file.append = mode != NULL && STRNEQ(mode, "append");
>      case VIR_DOMAIN_CHR_TYPE_PTY:
>      case VIR_DOMAIN_CHR_TYPE_DEV:
> -    case VIR_DOMAIN_CHR_TYPE_FILE:
>      case VIR_DOMAIN_CHR_TYPE_PIPE:
>          if (!path &&
>              def->type != VIR_DOMAIN_CHR_TYPE_PTY) {
> @@ -20051,10 +20056,15 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
>      case VIR_DOMAIN_CHR_TYPE_DEV:
>      case VIR_DOMAIN_CHR_TYPE_FILE:
>      case VIR_DOMAIN_CHR_TYPE_PIPE:
> +        /* 'append' makes sense only for files */
> +        if (def->type != VIR_DOMAIN_CHR_TYPE_FILE)
> +            def->data.file.append = false;
>          if (def->type != VIR_DOMAIN_CHR_TYPE_PTY ||
>              (def->data.file.path &&
>               !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))) {
> -            virBufferEscapeString(buf, "<source path='%s'",
> +            virBufferAsprintf(buf, "<source %s",
> +                          def->data.file.append ? "mode='append' " : "");
> +            virBufferEscapeString(buf, "path='%s'",
>                                    def->data.file.path);
>              virDomainSourceDefFormatSeclabel(buf, nseclabels, seclabels, flags);

We're already using the 'mode' attribute for a somewhat different
purpose on the <source> attribute with other types, so I think it
would be good practice to use a different attribute name. Probably
we should just use a virTristateBool for this, with an attribute
name of 'append'

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|