Re: [libvirt] [PATCH v4 4/7] storage: Add support to create a luks volume
On 07/15/2016 03:17 AM, Peter Krempa wrote:
On Thu, Jul 14, 2016 at 16:55:01 -0400, John Ferlan wrote:
>
>
> [...]
>
>>> +
>>> +void
>>> +virQEMUBuildLuksOpts(virBufferPtr buf,
>>> + virStorageEncryptionInfoDefPtr enc,
>>> + const char *alias)
>>> +{
>>> + virBufferAsprintf(buf, "key-secret=%s,", alias);
>>> +
>>> + /* If there's any cipher, then add that to the command line */
>>
>>> + if (enc->cipher_name) {
>>> + virBufferEscapeString(buf, "cipher-alg=%s-",
enc->cipher_name);
>>> + virBufferAsprintf(buf, "%u,", enc->cipher_size);
>>> + if (enc->cipher_mode)
>>> + virBufferEscapeString(buf, "cipher-mode=%s,",
>>> enc->cipher_mode);
>>> + if (enc->cipher_hash)
>>> + virBufferEscapeString(buf, "hash-alg=%s,",
>>> enc->cipher_hash);
>>> + if (enc->ivgen_name)
>>> + virBufferEscapeString(buf, "ivgen-alg=%s,",
>>> enc->ivgen_name);
>>> + if (enc->ivgen_hash)
>>> + virBufferEscapeString(buf, "ivgen-hash-alg=%s,",
>>> enc->ivgen_hash);
>>
>> s/virBufferEscapeString/qemuBufferEscapeComma/
>
> Not sure I understand what this is referencing.... Besides
I'd guess that it doesn't make much sense to escape < to < and > to
> in code that puts stuff on the command line rather to an XML and
that it makes more sense to escape a comma in the strings with two
commas as is usual for qemu command lines.
> qemuBufferEscapeComma is static to qemu_command
Extracting it to src/util/virbuffer.c could help.
Since it's QEMU specific I chose to put it in virqemu.c (there's patches
I posted today...)
I have to say, the result is rather ugly...
So these have gone from :
if (enc->cipher_name) {
virBufferAsprintf(buf, "cipher-alg=%s-%u,",
enc->cipher_name, enc->cipher_size);
if (enc->cipher_mode)
virBufferAsprintf(buf, "cipher-mode=%s,", enc->cipher_mode);
if (enc->cipher_hash)
virBufferAsprintf(buf, "hash-alg=%s,", enc->cipher_hash);
if (enc->ivgen_name)
virBufferAsprintf(buf, "ivgen-alg=%s,", enc->ivgen_name);
if (enc->ivgen_hash)
virBufferAsprintf(buf, "ivgen-hash-alg=%s,", enc->ivgen_hash);
}
to (assuming patches I've posted recently are accepted):
if (!enc->cipher_name)
return;
virBufferAddLit(buf, "cipher-alg=");
virQEMUBuildBufferEscapeComma(buf, enc->cipher_name);
virBufferAsprintf(buf, "-%u,", enc->cipher_size);
if (enc->cipher_mode) {
virBufferAddLit(buf, "cipher-mode=");
virQEMUBuildBufferEscapeComma(buf, enc->cipher_mode);
virBufferAddLit(buf, ",");
}
if (enc->cipher_hash) {
virBufferAddLit(buf, "hash-alg=");
virQEMUBuildBufferEscapeComma(buf, enc->cipher_hash);
virBufferAddLit(buf, ",");
}
if (!enc->ivgen_name)
return;
virBufferAddLit(buf, "cipher-name=");
virQEMUBuildBufferEscapeComma(buf, enc->ivgen_name);
virBufferAddLit(buf, ",");
if (enc->ivgen_hash) {
virBufferAddLit(buf, "ivgen-hash-alg=");
virQEMUBuildBufferEscapeComma(buf, enc->ivgen_hash);
virBufferAddLit(buf, ",");
}
All because someone could add a "," to one of those names...
John