<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Jan 30, 2014 at 9:46 AM, Ján Tomko <span dir="ltr"><<a href="mailto:jtomko@redhat.com" target="_blank">jtomko@redhat.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>On 01/23/2014 08:45 PM, Adam Walters wrote:<br>
> This patch fixes the secret type checking done in the<br>
> virDomainDiskDefParseXML function. Previously, it would not allow any<br>
> volumes that utilized a secret. This patch is a simple bypass of the<br>
> checking code for volumes.<br>
><br>
> Signed-off-by: Adam Walters <<a href="mailto:adam@pandorasboxen.com" target="_blank">adam@pandorasboxen.com</a>><br>
> ---<br>
>  src/conf/domain_conf.c | 6 ++++--<br>
>  1 file changed, 4 insertions(+), 2 deletions(-)<br>
><br>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c<br>
> index 28e24f9..773dc26 100644<br>
> --- a/src/conf/domain_conf.c<br>
> +++ b/src/conf/domain_conf.c<br>
> @@ -5418,7 +5418,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,<br>
>          cur = cur->next;<br>
>      }<br>
><br>
> -    if (auth_secret_usage != -1 && auth_secret_usage != expected_secret_usage) {<br>
> +    if (auth_secret_usage != -1 && auth_secret_usage != expected_secret_usage &&<br>
> +        def->type != VIR_DOMAIN_DISK_TYPE_VOLUME) {<br>
>          virReportError(VIR_ERR_INTERNAL_ERROR,<br>
>                         _("invalid secret type '%s'"),<br>
>                         virSecretUsageTypeTypeToString(auth_secret_usage));<br>
<br>
</div>So an rbd volume can have a secret when the pool has auth set to none?<br>
Otherwise it seems the volume's secret data might get overwritten by<br>
qemuTranslateDiskSourcePoolAuth.<br></blockquote><div><br></div><div>The purpose of this is to bypass the secret type checking for volumes (not just RBD volumes).</div><div><br></div><div>Above this section of code, but in the same function, there is some code that populates the</div>
<div>expected_secret_usage variable. Looking back on it now, I think I may have an alternative solution.</div><div>I think I might be able to set expected_secret_usage properly by referencing def->srcpool->pooltype</div>
<div>above this to check it like is done for non-storage pool RBD disks.</div><div><br></div><div>Without this particular patch hunk, any RBD volume used would throw an error in the logs:</div><div>"invalid secret type 'ceph'". If you didn't use a storage pool, but defined the RBD disk in the</div>
<div>domain XML directly with the secret instead, it worked just fine.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
And this could also be added to qemuxml2argvtest.<br></blockquote><div><br></div><div>I'll look into adding this into qemuxml2argvtest, as well.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div><br>
> @@ -18214,7 +18215,8 @@ virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,<br>
>      if (!disk->src || disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK ||<br>
>          (disk->type == VIR_DOMAIN_DISK_TYPE_VOLUME &&<br>
>           disk->srcpool &&<br>
> -         disk->srcpool->mode == VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT))<br>
> +         (disk->srcpool->mode == VIR_DOMAIN_DISK_SOURCE_POOL_MODE_DIRECT ||<br>
> +          disk->srcpool->mode == VIR_DOMAIN_DISK_TYPE_NETWORK)))<br>
<br>
</div>What is the purpose of this? You are comparing the source pool mode against a<br>
disk type constant. It seems this can never be true in this case.<br></blockquote><div><br></div><div>That was a typo. The second line should be disk->srcpool->actualtype == VIR_DOMAIN_DISK_TYPE_NETWORK))).</div><div>
I'll fix and submit a new version.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<br>
Jan<br>
<br>
</blockquote></div><br></div></div>