Hi, thanks for the review
I'd use a wording similar to what is used for copy&paste: "File transfer functionality (via Spice agent) is set using the <code>filetransfer</code> element. It is enabled by default, and can be disabled by setting the <code>copypaste</code> property to <code>no</code>", or at least I'd reword the first sentence which is a bit confusing imo, and is mostly implementation details. At this point, I think it will be "since 1.2.2"
fine for me, I will fix both.
virDomainGraphicsSpiceAgentFileTransferTypeFromString(enable)) <= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown enable value '%s'"), enable);
the code to disable copy&paste uses VIR_ERR_INTERNAL_ERROR when it's not able to parse the enum value, but the compression code uses VIR_ERR_CONFIG_UNSUPPORTED, and the mouse code uses VIR_ERR_XML_ERROR :( The rest of domain_conf.c is not very consistent in what error is reported on unknown enum values :( I'd personnally go with either VIR_ERR_CONFIG_UNSUPPORTED or VIR_ERR_XML_ERROR as they are more specific than VIR_ERR_INTERNAL_ERROR. However, given that the rest of the code is inconsistent, this can stay this way for now, and be fixed up at a later time.
I already have to resubmit, and I personally like errors to be specific, so I will change to VIR_ERR_CONFIG_UNSUPPORTED, because it seems the most relevant.
virDomainGraphicsSpiceAgentFileTransferTypeToString(def->data.spice.copypaste));
s/copypaste/filetransfer/
Ops :) will fix.
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU can't disable the spice file transfer"));
I'd say "spice file transfers" or "file transfers through spice" rather than "the spice file transfer"
Ok for "file transfers through spice". New version of the patch coming soon. Thanks and best regards, -- Francesco Romani