Re: [PATCH v4 4/7] storage: Add support to create a luks volume

[John Ferlan <jferlan@xxxxxxxxxx>]

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 &lt; and > to
> &gt; 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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list