On Tue, Jul 09, 2019 at 09:18:27AM -0700, Michal Skrivanek wrote:
> On 9 Jul 2019, at 15:37, Tomáš Golembiovský
<tgolembi(a)redhat.com> wrote:
>
> On Thu, 4 Jul 2019 23:59:07 +0300
> Nir Soffer <nsoffer(a)redhat.com> wrote:
>
>> On Tue, Jul 2, 2019 at 1:41 PM Tomáš Golembiovský <tgolembi(a)redhat.com>
>> wrote:
>>
>>> Hi,
>>>
>>> we have a large gap in handling ISO files in data domains and especially
>>> on block domains. None of the flows that work with ISOs works (well, one
>>> sort of does):
>>>
>>> 1) Change CD for running VM [1]
>>>
>>
>> Broken because VM.changeCD accepts a path instead of PDIV.
>>
>> This used to work with ISO domain since file based storage does not need
>> to be prepared, but with block storage someone must prepare the ISO image
>> before using it and tear down the image when done.
>>
>> If you send PDIV to vdsm vdsm will prepare the image and tear it down
>> and use the correct path to the image.
>>
>>
>>> 2) Attaching boot disk in Run Once -- this works but I wonder if it's
by
>>> accident, because engine issues spurious teardown on images [2]
>>>
>>
>> Can you point to more info about "spurious teardown"?
>
> See the linked bug [2]. When you configure same ISO to two VMs on the
> same host, engine tries to teardown the image after first VM is powered
> off while second VM is still using it. It is similar to the upload-flow
> you described below.
>
>
>>
>>> 3) Guest tools during VM import [3]
>>>
>>
>> Looks like [2] and [3] are the same. We don't know what
>> "Guest tools during VM import" means.
>
> When importing a VM user has an option to pick ISO with Windows drivers.
> The drivers will be injected into the VM during import. Same as 1) this
> relied on the fact that paths to ISO domain don't change and passed the
> path in API.
>
>
>>
>>> 4) Automatic attach of guest tools to Windows VMs
>>>
>>
>> I don't have any idea what is this automatic attach.
>
> Engine automatically picks guest tools ISO and adds it to the VM
> definition for you when you start Windows guest with old guest tools.
> Our main problem here is not related to prepare/teardown. We didn't get
> that far yet. :) Right now the issue is that the ISOs in data domains
> are not checked when searching for the latest ISO.
>
>
>>
>> Before virt can fix all those we need input from storage on how we
>>> should handle the images on block domains.
>>
>>
>> You should handle ISO images in the same way for all domains:
>> - never assume any path on the host, you should not care about the path,
>> and it is different for iso/data file/data block.
>> - If you start a vm with a ISO, or plug/unplug, always use PDIV and let vdsm
>> prepare and teardown the image for you, and update the xml with the
>> correct
>> path
>> - if you need to access the ISO externally (e.g. pass it to some virt tool)
>> you
>> probably want to prepare the image and get the path to the image on the
>> host
>> from the results of Image.prepare(). In this case you are responsible for
>> tearing
>> down the image. This how image transfer prepare and teardown images.
>>
>>
>>> The specialty of ISO image is
>>> that it makes sense to have it attached to multiple VMs.
>>
>>
>> There should be no issue to share ISO image as read only image.
>>
>>
>>> And that also
>>> means something should control whether image needs attaching or whether
>>> the image is already attached to the host.
>>
>>
>> All images need to be prepared and teardown. You should not care about the
>> type.
>> Vdsm will do the right thing.
>>
>>
>>> Similarly the image should be
>>> (attempted to) detached only when there is no longer any VM on the host
>>> that uses it.
>>>
>>
>> Usually engine is responsible for preparing and tearing down images. I know
>> we
>> have some holes in this area.
>>
>> Hence my question to storage -- is there already some framework from
>>> storage for handling this? Does it make more sense to do this on engine
>>> side or should each host handle it on it's own?
>>
>>
>> We considered moving management of prepared images to vdsm, in the same
>> way it handles managed block storage, but there are no concrete plans yet.
>>
>>> To me it seems
>>> preferable to handle this on engine side as that is where the requests
>>> for attach/detach normally come from (disks in VM import are an
>>> exception). It could possibly mean less race conditions (but I may be
>>> totally wrong here). Also having it on VDSM side would probably mean
>>> changes to the API for 1) and 2).
>>
>>
>> The issue is not where request to prepare and teardown come from, but
>> conflicting requests. For example:
>>
>> - Flow 1 prepare volume V1, V2, V3
>> - Flow 1 start to download volume V3
>> - Flow 2 prepare volumes V1, V2
>> - Flow 2 start to download volume V2
>> - Flow 1 finish download and tear down volumes V1, V2, V3
>> if you are lucky, volumes V1 and V2 fail to teardown, if you are not,
>> Flow 2 download fails
>> - Flow 2 finish and tear down volumes V1, V2
>> We have several requirements:
>> - If you prepare a volume, you must teardown
>> - volumes should be tore down only when there are no users
>>
>> This can be implemented in engine or vdsm, but since this is about host
>> internal state, I don't see why engine should care about it.
>
> So if I understand your comments right you are suggesting that we should
> ignore the fact that engine prepares/tears down images today and the
> logic should go to VDSM. But if I also understand things right there is
> no code in storage that would handle reference counting for us.
>
> So should we tie our changes to your changes in image management that
> you plan or can the ref. counting be done by storage in a separate
> effort?
I’m still rather in favor of “wasting” few MBs(well, 300 installed) on
hosts and getting rid of the need to upload and manage that iso in a
central location
But that only covers VM Import and possibly auto-attach for Windows. We
still need to figure out how to handle general case of attaching a CD to
VM -- both manual CD change and the Run Once flow.
Tomas
>
> >
> > Tomas
> >
> >>
> >> I hope it helps.
> >>
> >> Nir
> >>
> >> Any input appreciated.
> >>>
> >>>
> >>> Tomas
> >>>
> >>> [1]
https://bugzilla.redhat.com/show_bug.cgi?id=1589763
> >>> [2]
https://bugzilla.redhat.com/show_bug.cgi?id=1721455
> >>> [3]
https://bugzilla.redhat.com/show_bug.cgi?id=1721455
> >>>
> >>> --
> >>> Tomáš Golembiovský <tgolembi(a)redhat.com>
> >>>
> >
> >
> > --
> > Tomáš Golembiovský <tgolembi(a)redhat.com>
> > _______________________________________________
> > Devel mailing list -- devel(a)ovirt.org
> > To unsubscribe send an email to devel-leave(a)ovirt.org
> > Privacy Statement:
https://www.ovirt.org/site/privacy-policy/
> > oVirt Code of Conduct:
https://www.ovirt.org/community/about/community-guidelines/
> > List Archives:
https://lists.ovirt.org/archives/list/devel@ovirt.org/message/YTQTAV75R2R...