Re: [PATCH] qemu: Replace deprecated short-form boolean options

[Han Han <hhan@xxxxxxxxxx>]

On Tue, Jan 26, 2021 at 6:07 PM Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:

On 26/01/21 04:55, Han Han wrote:
> Since the commit ccd3b3b811 of QEMU, the short-form boolean options in
> qemu cmdline like "server", "nowait", "disable-ticketing" are
> deprecated

There are a few more:

1) -vnc password, -vnc tls, -vnc sasl:

     if (graphics->data.vnc.auth.passwd || cfg->vncPassword)
         virBufferAddLit(&opt, ",password");

     if (cfg->vncTLS) {
         qemuDomainGraphicsPrivatePtr gfxPriv =
QEMU_DOMAIN_GRAPHICS_PRIVATE(graphics);
         if (gfxPriv->tlsAlias) {
...
         } else {
             virBufferAddLit(&opt, ",tls");
             ...
         }
     }

     if (cfg->vncSASL) {
         virBufferAddLit(&opt, ",sasl");

         if (cfg->vncSASLdir)
             virCommandAddEnvPair(cmd, "SASL_CONF_PATH", cfg->vncSASLdir);

         /* TODO: Support ACLs later */
     }

"-vnc tls" is only used for old QEMU, but I think it's cleaner to change
it as well.

2) -chardev telnet

         virBufferAsprintf(&buf,
                           "socket,id=%s,host=%s,port=%s%s",
                           charAlias,
                           dev->data.tcp.host,
                           dev->data.tcp.service,
                           telnet ? ",telnet" : "");

3) -fsdev readonly:

     if (fs->readonly)
         virBufferAddLit(&opt, ",readonly");

4) -spice sasl:

     if (cfg->spiceSASL) {
         virBufferAddLit(&opt, "sasl,");

         if (cfg->spiceSASLdir)
             virCommandAddEnvPair(cmd, "SASL_CONF_PATH",
                                  cfg->spiceSASLdir);

         /* TODO: Support ACLs later */
     }

5) qemu-img create:

     if (info->features && info->format == VIR_STORAGE_FILE_QCOW2) {
         if (virBitmapIsBitSet(info->features,
                               VIR_STORAGE_FILE_FEATURE_LAZY_REFCOUNTS)) {
             if (STREQ_NULLABLE(info->compat, "0.10")) {
                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                                _("lazy_refcounts not supported with compat"
                                  " level %s"),
                                info->compat);
                 return -1;
             }
             virBufferAddLit(&buf, "lazy_refcounts,");
         }
     }

Hi Paolo,

I find there is no warning for the nolazy_refcounts option(qemu v5.2.0-1530-g74208cd252):

$ qemu-img create /tmp/b.qcow2 -f qcow2 10M -o nolazy_refcounts        
Formatting '/tmp/b.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=10485760 lazy_refcounts=off refcount_bits=16

Could you please help to check if this short-form boolean option is missing in the commit "ccd3b3b811    qemu-option: warn for short-form boolean options"

thanks

> diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c
> index 2d1f5ea5f5..97954bcc37 100644
> --- a/src/libxl/xen_common.c
> +++ b/src/libxl/xen_common.c
> @@ -872,7 +872,7 @@ xenParseSxprChar(const char *value,
>          else
>              def->source->data.tcp.service = g_strdup(offset);
> 
> -        if (offset2 && strstr(offset2, ",server"))
> +        if (offset2 && strstr(offset2, ",server=on"))
>              def->source->data.tcp.listen = true;
>      }
>      break;
> @@ -924,7 +924,7 @@ xenParseSxprChar(const char *value,
>              def->source->data.nix.path = g_strdup(value);
> 
>          if (offset != NULL &&
> -            strstr(offset, ",server") != NULL)
> +            strstr(offset, ",server=on") != NULL)
>              def->source->data.nix.listen = true;
>      }
>      break;

As far as I understand it, it is valid to start a domain with "xl" and
inspect it with "virsh dumpxml".  So I wouldn't change this, as it
depends on whatever xl has placed in the value you are parsing.

Thanks,

Paolo