[prev in list] [next in list] [prev in thread] [next in thread] 

List:       vdsm-devel
Subject:    =?utf-8?q?=5Bovirt-devel=5D?= Re: implementing hotplugCd/hotunplugCd in vdsm
From:       Michal Skrivanek <michal.skrivanek () redhat ! com>
Date:       2020-06-28 6:28:07
Message-ID: CAEZR_rUoMD6zD3v0syGEzWMVq-OjKxbM57NzhTTKg6HsYACnkg () mail ! gmail ! com
[Download RAW message or body]

Hi Fedor,

> On 27 Jun 2020, at 14:46, Fedor Gavrilov <fgavrilo@redhat.com> wrote:
> 
> So from what I was able to see in hotplugDisk example, for CDs I would have to do \
> similar: 
> 1. parse disk params from stored xml metadata
> 2. call prepareVolumePath with these params
> (skip normalizeVdsmImg)
> 3. call updateDriveIndex

Why?

> (skip validating whether volume has leases)
> 4. drive.getXML() and then hooks.before_disk_hotplug with it

If only the storage stuff would ever finish moving to xml....
But it's not right to call this hook anyway, iiuc you're not doing a hotplug

> 5. call attachDevice with drive XML and handle possible errors
> 
> I am not sure, however, whether we need what "else" does in hotplugDisk (vm.py line \
> 3468)... 
> Looking forward to hearing your opinions,
> Fedor
> 
> ----- Original Message -----
> From: "Fedor Gavrilov" <fgavrilo@redhat.com>
> To: "devel" <devel@ovirt.org>
> Sent: Monday, June 22, 2020 4:37:59 PM
> Subject: [ovirt-devel] implementing hotplugCd/hotunplugCd in vdsm
> 
> Hey,
> 
> So in an attempt to fix change CD functionality we discovered a few other potential

CD has a long history of issues caused by historical simplifications.
What is the intended fix, stop using string paths?

> issues and what Nir suggested was to implement two [somewhat] new functions in \
> VDSM: hotplug and hotunplug for CDs similar to how it works for normal disks now.

This is conceptually different, CD is a removable media,
hotplug/unplug is for the device itself. We never supported hotplug of
the device though.

> Existing changeCD function will be left as is for backwards compatibility.

Who would use the new function?

> As I found out, engine already calculates iface and index before invoking VDSM \
> functions, so we will just pass these along with PDIV to the VDSM. 
> Suggested flow is, let me quote:
> 
> > So the complete change CD flow should be:
> > 
> > 1. get the previous drivespec from vm metadata
> > 2. prepare new drivespec
> > 3. add new drivespec to vm metadata
> > 4. attach a new device to vm

Don't call it a new device when you just change a media

> > 5. teardown the previous drivespec
> > 6. remove previous drivespec from vm metadata
> > 
> > When the vm is stopped, it must do:
> > 
> > 1. get drive spec from vm metadata
> > 2. teardown drivespec
> > 
> > During attach, there are interesting races:
> > - what happens if vdsm crashes after step 2? who will teardown the volume?
> > maybe you need to add the new drivespec to the metadata first,
> > before preparing it.
> > - what happens if attach failed? who will remove the new drive from
> > the metadata?
> 
> Now, what makes hotplugDisk/hotunplugDisk different? From what I understand, the \
> flow is same there, so what difference is there as far as VDSM is concerned? If \
> none, this means if I more or less copy that code, changing minor details and data \
> accordingly for CDs, this should work, shouldn't it?

Hotplug/unplug is similar, but I would like to see the proposed change
in context of engine as well, and I expect it to be a bit more complex
there. Without that this is not worth it.

Thanks,
michal
> 
> Thanks!
> Fedor
> _______________________________________________
> Devel mailing list -- devel@ovirt.org
> To unsubscribe send an email to devel-leave@ovirt.org
> Privacy Statement: https://www.ovirt.org/privacy-policy.html
> oVirt Code of Conduct: https://www.ovirt.org/community/about/community-guidelines/
> List Archives: https://lists.ovirt.org/archives/list/devel@ovirt.org/message/LAQR3RW4RMTUNFUXL5T4HWLPKXJKEC3Y/
>  _______________________________________________
> Devel mailing list -- devel@ovirt.org
> To unsubscribe send an email to devel-leave@ovirt.org
> Privacy Statement: https://www.ovirt.org/privacy-policy.html
> oVirt Code of Conduct: https://www.ovirt.org/community/about/community-guidelines/
> List Archives: https://lists.ovirt.org/archives/list/devel@ovirt.org/message/VKPDAB7XXTQKPYWAZEHQYTV7TRCQQI2E/
> 
_______________________________________________
Devel mailing list -- devel@ovirt.org
To unsubscribe send an email to devel-leave@ovirt.org
Privacy Statement: https://www.ovirt.org/privacy-policy.html
oVirt Code of Conduct: https://www.ovirt.org/community/about/community-guidelines/
List Archives: https://lists.ovirt.org/archives/list/devel@ovirt.org/message/W2U55OKENZP2ZMNG7AJDYWE3NKBOPGOA/



[prev in list] [next in list] [prev in thread] [next in thread]