Hi Fedor,

> On 27 Jun 2020, at 14:46, Fedor Gavrilov <[email protected]> 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" <[email protected]>
> To: "devel" <[email protected]>
> 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 -- [email protected]
> To unsubscribe send an email to [email protected]
> 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/[email protected]/message/LAQR3RW4RMTUNFUXL5T4HWLPKXJKEC3Y/
> _______________________________________________
> Devel mailing list -- [email protected]
> To unsubscribe send an email to [email protected]
> 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/[email protected]/message/VKPDAB7XXTQKPYWAZEHQYTV7TRCQQI2E/
_______________________________________________
Devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]
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/[email protected]/message/W2U55OKENZP2ZMNG7AJDYWE3NKBOPGOA/

Reply via email to