On 17.07.2015 16:21, Michal Privoznik wrote:
> On 10.07.2015 08:20, Martin Kletzander wrote:
>> If one calls update-device with information that is not updatable,
>> libvirt reports success even though no data were updated. The example
>> used in the bug linked below uses updating device with <boot order='2'/>
>> which, in my opinion, is a valid thing to request from user's
>> perspective. Mainly since we properly error out if user wants to update
>> such data on a network device for example.
>>
>> And since there are many things that might happen (update-device on disk
>> basically knows just how to change removable media), check for what's
>> changing and moreover, since the function might be usable in other
>> drivers (updating only disk path is a valid possibility) let's abstract
>> it for any two disks.
>>
>> We can't possibly check for everything since for many fields our code
>> does not properly differentiate between default and unspecified values.
>> Even though this could be changed, I don't feel like it's worth the
>> complexity so it's not the aim of this patch.
>>
>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1007228
>> Signed-off-by: Martin Kletzander <mklet...@redhat.com>
>> ---
>>
>> Notes:
>> v2:
>> - Don't say 'NULL' when it should be 'unspecified'
>> - Don't blindly copy field name into the error message, but use space
>> instead of dot. That way it looks the same as in the XML provided.
>> - Check strings properly instead of addresses
>>
>> src/conf/domain_conf.c | 133
>> +++++++++++++++++++++++++++++++++++++++++++++++
>> src/conf/domain_conf.h | 2 +
>> src/libvirt_private.syms | 1 +
>> src/qemu/qemu_driver.c | 3 ++
>> 4 files changed, 139 insertions(+)
>>
>
>
> ACK
This was premature. You need to squash this in:
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2448a37..6562157 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5781,7 +5781,7 @@ virDomainDiskDiffersSourceOnly(virDomainDiskDefPtr disk,
"blkdeviotune size_iops_sec",
true);
- if (disk->transient && STRNEQ(disk->transient, orig_disk->transient)) {
+ if (disk->transient != orig_disk->transient) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_("cannot modify field '%s' of the disk"),
"transient");
disk->transient is a bool, not string.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
