virDomainDiskTranslateSourcePool: Don't set @mode of iscsi-direct

[libvirt] [PATCH] virDomainDiskTranslateSourcePool: Don't set @mode of iscsi-direct

Posted by Michal Privoznik 5 years, 1 month ago

https://bugzilla.redhat.com/show_bug.cgi?id=1658504

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/conf/domain_conf.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 05c2c9f34e..3340f0b0ef 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -30565,8 +30565,6 @@ virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def)
             return -1;
         }
 
-        def->src->srcpool->mode = VIR_STORAGE_SOURCE_POOL_MODE_DIRECT;
-
         if (virDomainDiskTranslateISCSIDirect(def, pooldef) < 0)
             return -1;
 
-- 
2.19.2

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

Re: [libvirt] [PATCH] virDomainDiskTranslateSourcePool: Don't set @mode of iscsi-direct

Posted by Erik Skultety 5 years, 1 month ago

On Mon, Mar 04, 2019 at 03:03:23PM +0100, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1658504

Some description apart from the bug would be appreciated.

>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/conf/domain_conf.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 05c2c9f34e..3340f0b0ef 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -30565,8 +30565,6 @@ virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def)
>              return -1;
>          }
>
> -        def->src->srcpool->mode = VIR_STORAGE_SOURCE_POOL_MODE_DIRECT;
> -

I think it might be worth enhancing the documentation on what does and what
does not support 'mode' attr. I think it's reasonable not to format
mode='direct' with iscsi-direct pool, it feels very redundant and a bit
confusing.

Reviewed-by: Erik Skultety <eskultet@redhat.com>

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

Re: [libvirt] [PATCH] virDomainDiskTranslateSourcePool: Don't set @mode of iscsi-direct

Posted by Michal Privoznik 5 years, 1 month ago

On 3/4/19 4:17 PM, Erik Skultety wrote:
> On Mon, Mar 04, 2019 at 03:03:23PM +0100, Michal Privoznik wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1658504
> 
> Some description apart from the bug would be appreciated.

D'oh! I intended to write some, this was just a placeholder commit 
message that I wrote in a hurry when leaving for the weekend. Of course 
I forgot that and when I've seen this branch I just sent the patch.

Fixing it now.

> 
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>   src/conf/domain_conf.c | 2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 05c2c9f34e..3340f0b0ef 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -30565,8 +30565,6 @@ virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def)
>>               return -1;
>>           }
>>
>> -        def->src->srcpool->mode = VIR_STORAGE_SOURCE_POOL_MODE_DIRECT;
>> -
> 
> I think it might be worth enhancing the documentation on what does and what
> does not support 'mode' attr. I think it's reasonable not to format
> mode='direct' with iscsi-direct pool, it feels very redundant and a bit
> confusing.

Consider it done. I'll add a comment there too.

> 
> Reviewed-by: Erik Skultety <eskultet@redhat.com>
> 

Thanks,
Michal

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