Couple of reasons for that:

a) there's no monitor command to change path where the pr-helper
connects to, or
b) there's no monitor command to introduce a new pr-helper for a
disk that already exists.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/libvirt_private.syms  |  1 +
 src/qemu/qemu_domain.c    |  8 ++++++++
 src/util/virstoragefile.c | 18 ++++++++++++++++++
 src/util/virstoragefile.h |  2 ++
 4 files changed, 29 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 071642c00..d4bff0f0b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2781,6 +2781,7 @@ virStorageNetHostTransportTypeToString;
 virStorageNetProtocolTypeToString;
 virStoragePRDefFormat;
 virStoragePRDefFree;
+virStoragePRDefIsEqual;
 virStoragePRDefParseNode;
 virStorageSourceBackingStoreClear;
 virStorageSourceClear;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 8b4efc82d..de8974d66 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7480,6 +7480,14 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
     CHECK_EQ(src->readonly, "readonly", true);
     CHECK_EQ(src->shared, "shared", true);
 
+    if (!virStoragePRDefIsEqual(disk->src->pr,
+                                orig_disk->src->pr)) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+                       _("cannot modify field '%s' of the disk"),
+                       "reservations");
+        return false;
+    }
+
 #undef CHECK_EQ
 
     return true;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 833d69f6d..328f59788 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2058,6 +2058,24 @@ virStoragePRDefFormat(virBufferPtr buf,
 }
 
 
+bool
+virStoragePRDefIsEqual(virStoragePRDefPtr a,
+                       virStoragePRDefPtr b)
+{
+    if (!a && !b)
+        return true;
+
+    if (!a || !b)
+        return false;
+
+    if (a->enabled != b->enabled ||
+        a->managed != b->managed ||
+        STRNEQ_NULLABLE(a->path, b->path))
+        return false;
+
+    return true;
+}
+
 virSecurityDeviceLabelDefPtr
 virStorageSourceGetSecurityLabelDef(virStorageSourcePtr src,
                                     const char *model)
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 968653660..e70c99076 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -383,6 +383,8 @@ virStoragePRDefPtr virStoragePRDefParseNode(xmlDocPtr xml,
                                             xmlNodePtr root);
 void virStoragePRDefFormat(virBufferPtr buf,
                            virStoragePRDefPtr prd);
+bool virStoragePRDefIsEqual(virStoragePRDefPtr a,
+                            virStoragePRDefPtr b);
 
 virSecurityDeviceLabelDefPtr
 virStorageSourceGetSecurityLabelDef(virStorageSourcePtr src,
-- 
2.16.1

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

On 02/21/2018 01:11 PM, Michal Privoznik wrote:
> Couple of reasons for that:
> 
> a) there's no monitor command to change path where the pr-helper
> connects to, or
> b) there's no monitor command to introduce a new pr-helper for a
> disk that already exists.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/libvirt_private.syms  |  1 +
>  src/qemu/qemu_domain.c    |  8 ++++++++
>  src/util/virstoragefile.c | 18 ++++++++++++++++++
>  src/util/virstoragefile.h |  2 ++
>  4 files changed, 29 insertions(+)
> 

[...]

> index 8b4efc82d..de8974d66 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -7480,6 +7480,14 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
>      CHECK_EQ(src->readonly, "readonly", true);
>      CHECK_EQ(src->shared, "shared", true);
>  
> +    if (!virStoragePRDefIsEqual(disk->src->pr,
> +                                orig_disk->src->pr)) {
> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> +                       _("cannot modify field '%s' of the disk"),
> +                       "reservations");

Formatting problems above - the " should line up I think... also
"diskreservations" will look odd.

John

> +        return false;
> +    }
> +
>  #undef CHECK_EQ
>  
>      return true;

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

On 03/02/2018 02:58 AM, John Ferlan wrote:
> 
> 
> On 02/21/2018 01:11 PM, Michal Privoznik wrote:
>> Couple of reasons for that:
>>
>> a) there's no monitor command to change path where the pr-helper
>> connects to, or
>> b) there's no monitor command to introduce a new pr-helper for a
>> disk that already exists.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  src/libvirt_private.syms  |  1 +
>>  src/qemu/qemu_domain.c    |  8 ++++++++
>>  src/util/virstoragefile.c | 18 ++++++++++++++++++
>>  src/util/virstoragefile.h |  2 ++
>>  4 files changed, 29 insertions(+)
>>
> 
> [...]
> 
>> index 8b4efc82d..de8974d66 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -7480,6 +7480,14 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
>>      CHECK_EQ(src->readonly, "readonly", true);
>>      CHECK_EQ(src->shared, "shared", true);
>>  
>> +    if (!virStoragePRDefIsEqual(disk->src->pr,
>> +                                orig_disk->src->pr)) {
>> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
>> +                       _("cannot modify field '%s' of the disk"),
>> +                       "reservations");
> 
> Formatting problems above - the " should line up I think... also
> "diskreservations" will look odd.

That's not what would be written. This is what would be:

error: cannot modify field 'reservations' of the disk

or translated:

fehler: Das Feld 'reservations' kann nicht geändert werden

IIRC it was discussed in v1 too. The idea is to have 'reservations' not
translated because it refers to the XML element name. However, the rest
of the error message can be localized.

Michal

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

On 03/06/2018 12:31 PM, Michal Privoznik wrote:
> On 03/02/2018 02:58 AM, John Ferlan wrote:
>>
>>
>> On 02/21/2018 01:11 PM, Michal Privoznik wrote:
>>> Couple of reasons for that:
>>>
>>> a) there's no monitor command to change path where the pr-helper
>>> connects to, or
>>> b) there's no monitor command to introduce a new pr-helper for a
>>> disk that already exists.
>>>
>>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>>> ---
>>>  src/libvirt_private.syms  |  1 +
>>>  src/qemu/qemu_domain.c    |  8 ++++++++
>>>  src/util/virstoragefile.c | 18 ++++++++++++++++++
>>>  src/util/virstoragefile.h |  2 ++
>>>  4 files changed, 29 insertions(+)
>>>
>>
>> [...]
>>
>>> index 8b4efc82d..de8974d66 100644
>>> --- a/src/qemu/qemu_domain.c
>>> +++ b/src/qemu/qemu_domain.c
>>> @@ -7480,6 +7480,14 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
>>>      CHECK_EQ(src->readonly, "readonly", true);
>>>      CHECK_EQ(src->shared, "shared", true);
>>>  
>>> +    if (!virStoragePRDefIsEqual(disk->src->pr,
>>> +                                orig_disk->src->pr)) {
>>> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
>>> +                       _("cannot modify field '%s' of the disk"),
>>> +                       "reservations");
>>
>> Formatting problems above - the " should line up I think... also
>> "diskreservations" will look odd.
> 
> That's not what would be written. This is what would be:
> 
> error: cannot modify field 'reservations' of the disk
> 
> or translated:
> 
> fehler: Das Feld 'reservations' kann nicht geändert werden
> 
> IIRC it was discussed in v1 too. The idea is to have 'reservations' not
> translated because it refers to the XML element name. However, the rest
> of the error message can be localized.
> 

Oh right - I missed the pesky '%s'...

John

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