Our code calls it when starting or re-starting the domain or when
hotplugging the disk so there's nothing to be detected.
---
 src/qemu/qemu_driver.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2032fac71..06bd442ee 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16508,9 +16508,6 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm,
         goto endjob;
     }

-    if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
-        goto endjob;
-
     /* clear the _SHALLOW flag if there is only one layer */
     if (!disk->src->backingStore)
         flags &= ~VIR_DOMAIN_BLOCK_COPY_SHALLOW;
@@ -16890,8 +16887,6 @@ qemuDomainBlockCommit(virDomainPtr dom,

     if (qemuDomainDiskBlockJobIsActive(disk))
         goto endjob;
-    if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
-        goto endjob;

     if (!top)
         topSource = disk->src;
-- 
2.12.0

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

On 03/15/2017 11:37 AM, Peter Krempa wrote:
> Our code calls it when starting or re-starting the domain or when
> hotplugging the disk so there's nothing to be detected.
> ---
>  src/qemu/qemu_driver.c | 5 -----
>  1 file changed, 5 deletions(-)
> 

I think I added it here because block jobs have the tendency to rewrite
the backing chain (think active commit, for example, which reduces the
backing chain).  Forcing a redetermination is therefore a way to ensure
no stale data associated with the pre-job chain is left in memory.

So I'm not sure this is correct.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

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

On Wed, Mar 22, 2017 at 14:21:29 -0500, Eric Blake wrote:
> On 03/15/2017 11:37 AM, Peter Krempa wrote:
> > Our code calls it when starting or re-starting the domain or when
> > hotplugging the disk so there's nothing to be detected.
> > ---
> >  src/qemu/qemu_driver.c | 5 -----
> >  1 file changed, 5 deletions(-)
> > 
> 
> I think I added it here because block jobs have the tendency to rewrite
> the backing chain (think active commit, for example, which reduces the
> backing chain).  Forcing a redetermination is therefore a way to ensure
> no stale data associated with the pre-job chain is left in memory.
> 
> So I'm not sure this is correct.

We re-detect the backing chain when the VM is started, when we reconnect
to it and when we get any of the blockjob-related events.

Additionally if the backing chain is rewritten the code would not be run
since the argument that would rewrite any previously detected data is
false.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 03/23/2017 02:13 AM, Peter Krempa wrote:
> On Wed, Mar 22, 2017 at 14:21:29 -0500, Eric Blake wrote:
>> On 03/15/2017 11:37 AM, Peter Krempa wrote:
>>> Our code calls it when starting or re-starting the domain or when
>>> hotplugging the disk so there's nothing to be detected.
>>> ---
>>>  src/qemu/qemu_driver.c | 5 -----
>>>  1 file changed, 5 deletions(-)
>>>
>>
>> I think I added it here because block jobs have the tendency to rewrite
>> the backing chain (think active commit, for example, which reduces the
>> backing chain).  Forcing a redetermination is therefore a way to ensure
>> no stale data associated with the pre-job chain is left in memory.
>>
>> So I'm not sure this is correct.
> 
> We re-detect the backing chain when the VM is started, when we reconnect
> to it and when we get any of the blockjob-related events.

Do we still care about qemu old enough where events did not appear, and
therefore where we had to fake events ourselves when we detect that the
job is complete? If so, then we want to redetect the chain (but only for
those older qemu); if not, then I can agree to your cleanup because
redetection at the event is sufficient.

> 
> Additionally if the backing chain is rewritten the code would not be run
> since the argument that would rewrite any previously detected data is
> false.
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

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

On Thu, Mar 23, 2017 at 10:31:06 -0500, Eric Blake wrote:
> On 03/23/2017 02:13 AM, Peter Krempa wrote:
> > On Wed, Mar 22, 2017 at 14:21:29 -0500, Eric Blake wrote:
> >> On 03/15/2017 11:37 AM, Peter Krempa wrote:
> >>> Our code calls it when starting or re-starting the domain or when
> >>> hotplugging the disk so there's nothing to be detected.
> >>> ---
> >>>  src/qemu/qemu_driver.c | 5 -----
> >>>  1 file changed, 5 deletions(-)
> >>>
> >>
> >> I think I added it here because block jobs have the tendency to rewrite
> >> the backing chain (think active commit, for example, which reduces the
> >> backing chain).  Forcing a redetermination is therefore a way to ensure
> >> no stale data associated with the pre-job chain is left in memory.
> >>
> >> So I'm not sure this is correct.
> > 
> > We re-detect the backing chain when the VM is started, when we reconnect
> > to it and when we get any of the blockjob-related events.
> 
> Do we still care about qemu old enough where events did not appear, and
> therefore where we had to fake events ourselves when we detect that the
> job is complete? If so, then we want to redetect the chain (but only for
> those older qemu); if not, then I can agree to your cleanup because
> redetection at the event is sufficient.

Since nobody complained that it would not work properly with such old
qemus I don't think that anybody uses it.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list