Incremental patches do look better. Just to make sure I am on the right track, I have some queries.
I have to apply the changes one function at a time, and these changes will be the same ones I made in the v2 and v3 patches, right?
If that is the case, do I need the next patch to be v4 or can the series of patches start from v1?
I can start afresh with the patches and this might save some confusion later.
Thanks.
On Wed, Apr 11, 2018 at 3:52 AM, John Ferlan <jferlan@xxxxxxxxxx> wrote:
On 04/02/2018 04:17 PM, Sukrit Bhatnagar wrote:
> This patch adds virQEMUBuildBufferEscapeComma to properly
> escape commas in user provided data fields for qemu command
> line processing.
>
> Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@xxxxxxxxx>
> ---
>
> Thank you for the helpful feedback and apologies for the delay.
Well we all get busy and delayed by other things! It's been a week since
you posted and well, I know I'm behind doing reviews!
Nice on the using the --- for your adjustments and the issue you discovered.
What happened to the changes from your previous version? They don't seem
to be included in this patch and they weren't pushed upstream. This
patch is all new changes.
The "next" step is to attempt to generate patches that make incremental
progress towards the end goal. Not all your changes need to go in one
pile especially if something is separable - there's a methodology one
develops over time. Changes don't need to be "so separated" that you get
very large series, but you can create smaller patches, altering single
API's/helpers and adjusting anything called by them to handle the
changes. Some times it's a changed result and other times it's doing
nothing because the patch fixes something. Again, it's one of those over
time things.
In this case, almost every function could have had it's own patch. That
way a reviewer can ACK/Reviewed-by and push part of the series while
perhaps asking for respins on other parts. It also allows for a NACK of
a specific area. Much easier to change and review smaller things too!
Since this is a GSOC type activity I won't "do" the work for you, but I
will help "guide" you to the answer. First things first - hopefully you
haven't lost your original patch. It's easily recreateable at least. We
will start easy/slow... Using that patch as a starting point, create a
series of 5 patches
patch 1: Changes for qemuBuildRomStr
patch 2: Changes for qemuBuildDriveDevStr
patch 3: Changes for qemuBuildFSStr and qemuBuildFSDevStr
patch 4: Changes for qemuBuildGraphicsVNCCommandLine
patch 5: Changes for qemuBuildDomainLoaderCommandLine
Hold onto the changes for qemuBuildHostNetStr,
qemuBuildChrChardevFileStr, qemuBuildChrChardevStr, and
qemuBuildGraphicsSPICECommandLine as they'll be combined with separated
patches from this patch.
Post the above 5 - I've included patch 1 & 2 for you as an attachment to
this reply so you can see the format... It should be fairly easy to copy
from there and post as a v4.
Once you've done that - work through the rest of this using similar
context - although a few of these will be a bit larger and more
complicated (especially the first one for build network drive.
This is because qemuBuildDriveSourceStr calls qemuGetDriveSourceString
>
> Changes in v3:
> virQEMUBuildBufferEscapeComma was applied to:
> - src->hosts->socket in qemuBuildNetworkDriveURI
> - src->path, src->configFile in qemuBuildNetworkDriveStr
> - disk->blkdeviotune.group_name in qemuBuildDiskThrottling
> - net->data.socket.address, net->data.socket.localaddr in
> qemuBuildHostNetStr
> - dev->data.file.path in qemuBuildChrChardevStr
> - graphics->data.spice.rendernode in
> qemuBuildGraphicsSPICECommandLine
> - smartcard->data.cert.file[i], smartcard->data.cert.database in
> qemuBuildSmartcardCommandLine
>
> Changes in v2:
> virQEMUBuildBufferEscapeComma was applied to:
> - info->romfile in qemuBuildRomStr
> - disk->vendor, disk->product in qemuBuildDriveDevStr
> - fs->src->path in qemuBuildFSStr
> - fs->dst in qemuBuildFSDevStr
> - connect= in qemuBuildHostNetStr
> - fileval handling in qemuBuildChrChardevStr
> - TYPE_DEV, TYPE_PIPE handling in qemuBuildChrChardevStr
> - cfg->vncTLSx509certdir in qemuBuildGraphicsVNCCommandLine
> - cfg->spiceTLSx509certdir in qemuBuildGraphicsSPICECommandLine
> - loader->path, loader->nvram usage in
> qemuBuildDomainLoaderCommandLine
>
> Link to v2: https://www.redhat.com/archives/libvir-list/2018- March/msg00965.html
>
>
> When I tried to change src->path in qemuBuildNetworkDriveStr
> for this portion
>
> 961 } else if (src->nhosts == 1) {
> 962 if (virAsprintf(&ret, "sheepdog:%s:%u:%s",
> 963 src->hosts->name, src->hosts->port,
> 964 src->path) < 0)
> 965 goto cleanup;
> 966 } else {
>
> make check reported the following error.
>
> 141) QEMU XML-2-ARGV disk-drive-network-sheepdog ...
> In '/home/skrtbhtngr/libvirt/tests/qemuxml2argvdata/disk- drive-network-sheepdog.args':
> Offset 0
> Expect [LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none /usr/bin/qemu-system-i686 -name QEMUGuest1 -S -M pc -m 214 -smp 1,sockets=1,cores=1,threads=1 -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 -nographic -nodefaults -chardev socket,id=charmonitor,path=/ tmp/lib/domain--1-QEMUGuest1/ monitor.sock,server,nowait -mon chardev=charmonitor,id= monitor,mode=readline -no-acpi -boot c -usb -drive file=/dev/HostVG/QEMU,,Guest,, ,,1,format=raw,if=none,id= drive-ide0-0-0 -device ide-drive,bus=ide.0,unit=0, drive=drive-ide0-0-0,id=ide0- 0-0 -drive file=sheepdog:example.org:6000 :image,,with,,commas,format= raw,if=none,id=drive-virtio- disk0 -device virtio-blk-pci,bus=pci.0,addr= 0x3,drive=drive-virtio-disk0, id=virtio-disk0
> ]
> Actual [LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none /usr/bin/qemu-system-i686 -name QEMUGuest1 -S -M pc -m 214 -smp 1,sockets=1,cores=1,threads=1 -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 -nographic -nodefaults -chardev socket,id=charmonitor,path=/ tmp/lib/domain--1-QEMUGuest1/ monitor.sock,server,nowait -mon chardev=charmonitor,id= monitor,mode=readline -no-acpi -boot c -usb -drive file=/dev/HostVG/QEMU,,Guest,, ,,1,format=raw,if=none,id= drive-ide0-0-0 -device ide-drive,bus=ide.0,unit=0, drive=drive-ide0-0-0,id=ide0- 0-0 -drive file=sheepdog:example.org:6000 :image,,,,with,,,,commas, format=raw,if=none,id=drive- virtio-disk0 -device virtio-blk-pci,bus=pci.0,addr= 0x3,drive=drive-virtio-disk0, id=virtio-disk0
> ]
> ... FAILED
>
>
> In disk-drive-network-sheepdog.args:
> ...
> -drive file=sheepdog:example.org:6000:image,,with,,commas,format= raw,if=none,\
> ...
>
> I was not quite sure how to handle this part. Adding
> virQEMUBuildBufferEscapeComma there is escaping the twin commas
> in the file name again. I have left that part unchanged.
>
which calls qemuBuildNetworkDriveStr returning @source. Then back in
qemuBuildDriveSourceStr the @source goes through another transformation:
if (source) {
virBufferAddLit(buf, "file=");
...
virQEMUBuildBufferEscapeComma(buf, source);
...
Because the return from qemuBuildNetworkDriveStr is used by callers that
don't expect qemuGetDriveSourceString to return a comma escaped string
that means adding a bit of logic so that qemuBuildNetworkDriveURI and
qemuBuildNetworkDriveStr can escape only when necessary.
Returning an escaped string for qemuDomainSnapshotCreateSingleDiskActive
would not be a good thing.
Still it's a *good thing* you've gone through this exercise *and* made
note of what you saw. It makes it clearer what the "right" path is for
me at least. Of course if you'd separated out your patches it would make
resolving it a bit easier!
Also, this exercise has shown there's a bug in how the command line is
built for hostdev's. The source is not escaped, although I doubt we'd
get an iSCSI tgt/lun with a "rogue" comma - it's just not expected.
Still who knows, we still need to handle it.
>
> src/qemu/qemu_command.c | 111 +++++++++++++++++++++++++----------------------- I think a third parameter "bool escape" will be necessary...
> 1 file changed, 59 insertions(+), 52 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 6f76f18ab..26b36551c 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -844,14 +844,18 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
> qemuDomainSecretInfoPtr secinfo)
> {
> virURIPtr uri = NULL;
> - char *ret = NULL;
> + char *ret = NULL, *socket = NULL;
There is a general preference for one argument per line.
> + virBuffer buf = VIR_BUFFER_INITIALIZER;
>
> if (!(uri = qemuBlockStorageSourceGetURI(src))) This logic needs to be separated into "if (escape)" escape the socket, e.g.:
> goto cleanup;
>
> - if (src->hosts->socket &&
> - virAsprintf(&uri->query, "socket=%s", src->hosts->socket) < 0)
> - goto cleanup;
> + if (src->hosts->socket) {
> + virQEMUBuildBufferEscapeComma(&buf, src->hosts->socket);
> + socket = virBufferContentAndReset(&buf);
> + if (virAsprintf(&uri->query, "socket=%s", socket) < 0)
> + goto cleanup;
> + }
if (src->hosts->socket) {
if (escape) {
virQEMUBuildBufferEscapeComma(&buf, src->hosts->socket);
socket = virBufferContentAndReset(&buf);
}
if (virAsprintf(&uri->query, "socket=%s",
socket ? socket : src->hosts->socket) < 0)
goto cleanup;
}
>
> if (qemuBuildGeneralSecinfoURI(uri, secinfo) < 0) The virBufferContentAndReset will reinitialize the buffer, so no need
> goto cleanup;
> @@ -860,6 +864,8 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
>
> cleanup:
> virURIFree(uri);
> + virBufferFreeAndReset(&buf);
> +
for this call, but we would leak @socket possibly, so that does need to
be freed.... Also, need for extra blank line here. This then is just:
VIR_FREE(socket);
> return ret;
> }
>
> @@ -868,8 +874,9 @@ static char *
> qemuBuildNetworkDriveStr(virStorageSourcePtr src, I think a third parameter "bool escape" will be necessary...
> qemuDomainSecretInfoPtr secinfo)
> {
> - char *ret = NULL;
> + char *ret = NULL, *path = NULL, *file = NULL;
again, one line and we should only need one, e.g. 'tmp' - since we know
it's initialized to NULL we can use that when deciding whether we escape
or not.
> virBuffer buf = VIR_BUFFER_INITIALIZER;
> + virBuffer bufTemp = VIR_BUFFER_INITIALIZER;
> size_t i;
>
> switch ((virStorageNetProtocol) src->protocol) {
> @@ -914,8 +921,10 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src, if (src->path) {
> goto cleanup;
> }
>
> - if (src->path)
> - virBufferAsprintf(&buf, ":exportname=%s", src->path);
> + if (src->path) {
> + virBufferAddLit(&buf, ":exportname=");
> + virQEMUBuildBufferEscapeComma(&buf, src->path);
> + }
if (escape) {
virBufferAddLit(&buf, ":exportname=");
virQEMUBuildBufferEscapeComma(&buf, src->path);
} else {
virBufferAsprintf(&buf, ":exportname=%s", src->path);
}
}
>
> if (virBufferCheckError(&buf) < 0)
> goto cleanup;
> @@ -945,7 +954,9 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src, if (escape) {
> }
>
> if (src->nhosts == 0) {
> - if (virAsprintf(&ret, "sheepdog:%s", src->path) < 0)
> + virQEMUBuildBufferEscapeComma(&bufTemp, src->path);
> + path = virBufferContentAndReset(&bufTemp);
> + if (virAsprintf(&ret, "sheepdog:%s", path) < 0)
> goto cleanup;
> } else if (src->nhosts == 1) {
> if (virAsprintf(&ret, "sheepdog:%s:%u:%s",
virQEMUBuildBufferEscapeComma(&bufTemp, src->path);
tmp = virBufferContentAndReset(&bufTemp);
}
if (src->nhosts == 0) {
if (virAsprintf(&ret, "sheepdog:%s", tmp ? tmp : src->path) < 0)
goto cleanup;
} else if (src->nhosts == 1) {
if (virAsprintf(&ret, "sheepdog:%s:%u:%s",
src->hosts->name, src->hosts->port,
tmp ? tmp : src->path) < 0)
goto cleanup;
} else {
NB: In your patch - if the .xml/.args file hadn't used a host w/ a path
having a comma, then that would have failed.
IOW: if tests/qemuxml2argdata/disk-drive-network-sheepdog.xml:
<disk type='network' device='disk'>
<driver name='qemu' type='raw'/>
<source protocol='sheepdog' name='image,with,commas'>
<host name='example.org' port='6000'/>
</source>
<target dev='vda' bus='virtio'/>
</disk>
was:
<disk type='network' device='disk'>
<driver name='qemu' type='raw'/>
<source protocol='sheepdog' name='image,with,commas'/>
<target dev='vda' bus='virtio'/>
</disk>
then tests/qemuxml2argvdata/disk-drive-network-sheepdog.args would
change from:
-drive
file=sheepdog:example.org:6000:image,,with,,commas,format= id=drive-virtio-disk0 \raw,if=none,\
to (something like):
-drive file=sheepdog:image,,with,,commas,format=raw,if=none,\
id=drive-virtio-disk0 \
But with your change it would have had the 4 commas (hopefully this
makes sense).
> @@ -967,8 +978,9 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src, if (escape) {
> src->path);
> goto cleanup;
> }
> -
> - virBufferStrcat(&buf, "rbd:", src->volume, "/", src->path, NULL);
> + virQEMUBuildBufferEscapeComma(&bufTemp, src->path);
> + path = virBufferContentAndReset(&bufTemp);
> + virBufferStrcat(&buf, "rbd:", src->volume, "/", path, NULL);
virQEMUBuildBufferEscapeComma(&bufTemp, src->path);
tmp = virBufferContentAndReset(&bufTemp);
}
virBufferStrcat(&buf, "rbd:", src->volume, "/",
tmp ? tmp : src->path, NULL);
VIR_FREE(tmp);
>
> if (src->snapshot)
> virBufferEscape(&buf, '\\', ":", "@%s", src->snapshot);
> @@ -994,8 +1006,11 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src, if (src->configFile) {
> }
> }
>
> - if (src->configFile)
> - virBufferEscape(&buf, '\\', ":", ":conf=%s", src->configFile);
> + if (src->configFile) {
> + virQEMUBuildBufferEscapeComma(&bufTemp, src->configFile);
> + file = virBufferContentAndReset(&bufTemp);
> + virBufferEscape(&buf, '\\', ":", ":conf=%s", file);
> + }
if (escape) {
virQEMUBuildBufferEscapeComma(&bufTemp, src->configFile);
tmp = virBufferContentAndReset(&bufTemp);
}
virBufferEscape(&buf, '\\', ":", ":conf=%s",
tmp ? tmp : src->configFile);
}
>
> if (virBufferCheckError(&buf) < 0)
> goto cleanup;
> @@ -1022,6 +1037,7 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src, Again, each of the callers used virBufferContentAndReset, so the @path
> }
>
> cleanup:
> + virBufferFreeAndReset(&bufTemp);
and @file arguments would have been needed to be VIR_FREE'd instead.
However, if you take my suggestion w/ a tmp variable, then you just
VIR_FREE(tmp) instead.
> virBufferFreeAndReset(&buf);
>
> return ret;
When you post your next patch - use this opportunity to separate out
these two functions into their own patch (along with adjustments to
callers to pass the escape bool. This will be the most complex patch
out of them all.
> @@ -1630,6 +1646,8 @@ qemuBuildDiskThrottling(virDomainDiskDefPtr disk, These can move to ...
> virBufferAsprintf(buf, ",throttling." _label "=%llu", \
> disk->blkdeviotune._field); \
> }
> + virBuffer bufTemp = VIR_BUFFER_INITIALIZER;
> + char *name = NULL;
>
> IOTUNE_ADD(total_bytes_sec, "bps-total");
> IOTUNE_ADD(read_bytes_sec, "bps-read");
> @@ -1647,8 +1665,9 @@ qemuBuildDiskThrottling(virDomainDiskDefPtr disk, ... here (IOW: localize them more).
>
> IOTUNE_ADD(size_iops_sec, "iops-size");
> if (disk->blkdeviotune.group_name) {
> - virBufferEscapeString(buf, ",throttling.group=%s",
> - disk->blkdeviotune.group_name); VIR_FREE(name);
> + virQEMUBuildBufferEscapeComma(&bufTemp, disk->blkdeviotune.group_name) ;
> + name = virBufferContentAndReset(&bufTemp);
> + virBufferEscapeString(buf, ",throttling.group=%s", name);
> }
>
> IOTUNE_ADD(total_bytes_sec_max_length, "bps-total-max-length"); NB: virBufferContentAndReset will be all you need for bufTemp, but @name
> @@ -1657,6 +1676,8 @@ qemuBuildDiskThrottling(virDomainDiskDefPtr disk,
> IOTUNE_ADD(total_iops_sec_max_length, "iops-total-max-length");
> IOTUNE_ADD(read_iops_sec_max_length, "iops-read-max-length");
> IOTUNE_ADD(write_iops_sec_max_length, "iops-write-max-length");
> +
> + virBufferFreeAndReset(&bufTemp);
is leaked, but that's adjusted above
> #undef IOTUNE_ADD
> }
>
The changes for qemuBuildDiskThrottling should be in one patch.
> @@ -3651,27 +3672,25 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, This has alignment issues w/ the previous line.
> break;
>
> case VIR_DOMAIN_NET_TYPE_SERVER:
> - virBufferAsprintf(&buf, "socket%clisten=%s:%d,",
> - type_sep,
> + virBufferAsprintf(&buf, "socket%clisten=", type_sep);
> + virQEMUBuildBufferEscapeComma(&buf,
> net->data.socket.address ? net->data.socket.address
> - : "",
> - net->data.socket.port);
> + : "");
It also points out something I'm not sure whether it's a bug or not. If
net->data.socket.address == NULL, then the command line would be (from
net-vhostuser.args):
-netdev socket,listen=:2015,
But that looks strange to me, I guess I'd expect:
-netdev socket,listen="":2015,
Still the former syntax works so I suppose it's OK.
Still changes could be rewritten as:
virBufferAsprintf(&buf, "socket%clisten=", type_sep);
virQEMUBuildBufferEscapeComma(&buf, net->data.socket.address);
virBufferAsprintf(&buf, ":%d,", net->data.socket.port);
BTW: Your previous patch added a couple of changes to this API - they
should be moved into here so that we have all the adjustments to
qemuBuildHostNetStr in one patch.
And like noted before - the above hunk can be it's own patch!
> + virBufferAsprintf(&buf, ":%d,", net->data.socket.port);
> break;
>
> case VIR_DOMAIN_NET_TYPE_MCAST:
> - virBufferAsprintf(&buf, "socket%cmcast=%s:%d,",
> - type_sep,
> - net->data.socket.address,
> - net->data.socket.port);
> + virBufferAsprintf(&buf, "socket%cmcast=", type_sep);
> + virQEMUBuildBufferEscapeComma(&buf, net->data.socket.address);
> + virBufferAsprintf(&buf, ":%d,", net->data.socket.port);
> break;
>
> case VIR_DOMAIN_NET_TYPE_UDP:
> - virBufferAsprintf(&buf, "socket%cudp=%s:%d,localaddr=%s:%d,",
> - type_sep,
> - net->data.socket.address,
> - net->data.socket.port,
> - net->data.socket.localaddr,
> - net->data.socket.localport);
> + virBufferAsprintf(&buf, "socket%cudp=", type_sep);
> + virQEMUBuildBufferEscapeComma(&buf, net->data.socket.address);
> + virBufferAsprintf(&buf, ":%d,localaddr=", net->data.socket.port);
> + virQEMUBuildBufferEscapeComma(&buf, net->data.socket.localaddr);
> + virBufferAsprintf(&buf, ":%d,", net->data.socket.localport);
> break;
>
> case VIR_DOMAIN_NET_TYPE_USER:
> @@ -4954,9 +4973,10 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager, One line per argument...
> bool chardevStdioLogd)
> {
> virBuffer buf = VIR_BUFFER_INITIALIZER;
> + virBuffer bufTemp = VIR_BUFFER_INITIALIZER;
> bool telnet;
> char *charAlias = NULL;
> - char *ret = NULL;
> + char *ret = NULL, *path = NULL;
>
> if (!(charAlias = qemuAliasChardevFromDevAlias(alias))) path is leaked.
> goto cleanup;
> @@ -4990,9 +5010,11 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
> _("append not supported in this QEMU binary"));
> goto cleanup;
> }
> + virQEMUBuildBufferEscapeComma(&bufTemp, dev->data.file.path);
> + path = virBufferContentAndReset(&bufTemp);
> if (qemuBuildChrChardevFileStr(chardevStdioLogd ? logManager : NULL,
> cmd, def, &buf,
> - "path", dev->data.file.path,
> + "path", path,
> "append", dev->data.file.append) < 0)
> goto cleanup;
> break;
And the above is in it's own patch. Here again, I'd combine the changes
from your original patch to qemuBuildChrChardevStr into this one. I'd
also include the changes for qemuBuildChrChardevFileStr within the same
patch since they are "related".
> @@ -8150,8 +8172,8 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, The above can be it's own patch and of course include the SPICE change
> _("This QEMU doesn't support spice OpenGL rendernode"));
> goto error;
> }
> -
> - virBufferAsprintf(&opt, "rendernode=%s,", graphics->data.spice.rendernode);
> + virBufferAddLit(&opt, "rendernode=");
> + virQEMUBuildBufferEscapeComma(&opt, graphics->data.spice. rendernode);
> }
> }
>
from your original patch too.
And this one gets it's own patch too.
> @@ -8771,7 +8793,6 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager,
> virDomainSmartcardDefPtr smartcard;
> char *devstr;
> virBuffer opt = VIR_BUFFER_INITIALIZER;
> - const char *database;
> const char *contAlias = NULL;
>
> if (!def->nsmartcards)
> @@ -8814,29 +8835,15 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager,
>
> virBufferAddLit(&opt, "ccid-card-emulated,backend=certificates");
> for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) {
> - if (strchr(smartcard->data.cert.file[i], ',')) {
> - virBufferFreeAndReset(&opt);
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> - _("invalid certificate name: %s"),
> - smartcard->data.cert.file[i]);
> - return -1;
> - }
> - virBufferAsprintf(&opt, ",cert%zu=%s", i + 1,
> - smartcard->data.cert.file[i]);
> + virBufferAsprintf(&opt, ",cert%zu=", i + 1);
> + virQEMUBuildBufferEscapeComma(&opt, smartcard->data.cert.file[i]);
> }
> if (smartcard->data.cert.database) {
> - if (strchr(smartcard->data.cert.database, ',')) {
> - virBufferFreeAndReset(&opt);
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> - _("invalid database name: %s"),
> - smartcard->data.cert.database);
> - return -1;
> - }
> - database = smartcard->data.cert.database;
> + virBufferAddLit(&opt, ",db=");
> + virQEMUBuildBufferEscapeComma(&opt, smartcard->data.cert.database) ;
> } else {
> - database = VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE;
> + virBufferAsprintf(&opt, ",db=%s", VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE);
> }
> - virBufferAsprintf(&opt, ",db=%s", database);
> break;
>
> case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
>
In the end, probably 11 patches total. Do the easy ones first. It's
always good to make some progress and have some success rather than
having to redo everything all over again and have this large pile of a
singlular change waiting for some part of it to be fixed. Once
everything is done we can remove the entry from bite sized tasks.
John
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
