6:02 a.m.
https://bugzilla.redhat.com/show_bug.cgi?id=1159219
So, in 11e058ca589808bd I've tried to make UpdateDevice update
startupPolicy too. And it worked well until somebody came around
and pushed d0dc6c036914da which accidentally removed my
contribution. Redo my commit.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_driver.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 91eb661..03ef972 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8353,6 +8353,7 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps,
* We allow updating src/type//driverType/cachemode/
*/
orig->cachemode = disk->cachemode;
+ orig->startupPolicy = disk->startupPolicy;
virStorageSourceFree(orig->src);
orig->src = disk->src;
--
2.4.6
6:24 a.m.
On Mon, Sep 14, 2015 at 13:02:49 +0200, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1159219
So, in 11e058ca589808bd I've tried to make UpdateDevice update
startupPolicy too. And it worked well until somebody came around
and pushed d0dc6c036914da which accidentally removed my
contribution. Redo my commit.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_driver.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 91eb661..03ef972 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8353,6 +8353,7 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps,
* We allow updating src/type//driverType/cachemode/
*/
orig->cachemode = disk->cachemode;
+ orig->startupPolicy = disk->startupPolicy;
This also might make sense to be updated in the "live" case to
"requisite" setting for migration.
Peter
7 a.m.
On 14.09.2015 13:24, Peter Krempa wrote:
On Mon, Sep 14, 2015 at 13:02:49 +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1159219
>
> So, in 11e058ca589808bd I've tried to make UpdateDevice update
> startupPolicy too. And it worked well until somebody came around
> and pushed d0dc6c036914da which accidentally removed my
> contribution. Redo my commit.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/qemu/qemu_driver.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 91eb661..03ef972 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -8353,6 +8353,7 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps,
> * We allow updating src/type//driverType/cachemode/
> */
> orig->cachemode = disk->cachemode;
> + orig->startupPolicy = disk->startupPolicy;
This also might make sense to be updated in the "live" case to
"requisite" setting for migration.
I think not only that. In fact all three options make sense on live case
too. mandatory - I want to fail migration if disk is not present on dst,
requisite is the same as optional in this case, since if domain is
started up, it means that disk is there, it's present. But I might want
to drop it during migration.
Okay, I'll repost.
Michal
9:18 a.m.
On 14.09.2015 14:00, Michal Privoznik wrote:
On 14.09.2015 13:24, Peter Krempa wrote:
> On Mon, Sep 14, 2015 at 13:02:49 +0200, Michal Privoznik wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1159219
>>
>> So, in 11e058ca589808bd I've tried to make UpdateDevice update
>> startupPolicy too. And it worked well until somebody came around
>> and pushed d0dc6c036914da which accidentally removed my
>> contribution. Redo my commit.
>>
>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
>> ---
>> src/qemu/qemu_driver.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 91eb661..03ef972 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -8353,6 +8353,7 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps,
>> * We allow updating src/type//driverType/cachemode/
>> */
>> orig->cachemode = disk->cachemode;
>> + orig->startupPolicy = disk->startupPolicy;
>
> This also might make sense to be updated in the "live" case to
> "requisite" setting for migration.
I think not only that. In fact all three options make sense on live case
too. mandatory - I want to fail migration if disk is not present on dst,
requisite is the same as optional in this case, since if domain is
started up, it means that disk is there, it's present. But I might want
to drop it during migration.
Okay, I'll repost.
Hm, now that I dig into the code, it's not going to be as trivial as this one. So far
in the live update we allow only disk->src to change. We even have a function to check
that and reject changing anything else: virDomainDiskDiffersSourceOnly (not to mention,
that function does not check many fields).
Anyway, I am not sure what approach to take. Maybe I could turn the
virDomainDiskDiffersSourceOnly into qemuDomainDiskChangeSupported which would reject
updating all the unsupported fields within virDomainDiskDef. But then I'm going to
need virDomainDiskDiffersSource or how to name it, that will return true if disk source
needs updating. Then I'm going to need "a function" (bare 'if' would
be enough) to check on startupPolicy change. Something like following:
static int
qemuDomainChangeDiskMediaLive()
{
switch (disk->device) {
case VIR_DOMAIN_DISK_DEVICE_CDROM:
case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def,
disk->bus, disk->dst)))
goto end;
if (!qemuDomainDiskChangeSupported(disk, orig_disk)
goto end;
if (virDomainDiskDiffersSource(disk, orig_disk) {
change_media();
}
if (disk->startupPolicy != orig_disk) {
orig_disk->startupPolicy = disk->startupPolicy;
}
ret = 0;
}
}
My question is, does this sound reasonable? Can this patch be taken in, while I'm
working on the live part?
Michal
3357
days inactive
3357
days old
3 comments
2 participants
participants (2)
-
Michal Privoznik -
Peter Krempa