On 29. 3. 2020 0:43, Martin Kletzander wrote: > On Wed, Mar 25, 2020 at 02:59:16PM +0100, Michal Prívozník wrote: >> On 23. 3. 2020 10:45, Prathamesh Chavan wrote: >>> Hi Everyone, >> >> Hey! >> >> It's nice to see somebody new interested in libvirt. >> >>> >>> I'm Prathamesh Chavan, a final year student at studying Computer >>> Science and Engineering at IIT Kharagpur. I've been part of GSoC'17 >>> under the Git Organization and have an internship experience at >>> Nutanix, India during last summer. >>> I'm also currently working on a tired file system with software wear >>> management for nvm technologies as my master project. >>> I was interested in contributing to the project: "Introducing job >>> control to the storage driver" under Google Summer of Code - 2020. >>> I was currently going through the codebase and also was successfully >>> able to build it. >>> It would be great if I was assigned a byte-task regarding the same >>> which could help me understand the project better, >> >> There is no assignment process, just pick anything you want. Adapting to >> GLib is pretty popular as it usually removes some lines :-) But feel >> free to pick anything you would like. >> >> And for the GSoC project itself; we currently have in_use member for >> virStorageVolDef structure which serves as a refcounter. Then we have >> some mutexes to guard access to the counter. Whenever a 'long running' >> storage API is called (e.g. virStorageVolWipe(), >> virStorageVolDownload(), ...) the refcounter is increased at the >> beginning and then decreased at the end. Then, virStorageVolDelete() >> checks this refcounter to see if there is some other thread using it. >> >> But we already have something similar implemented for virDomainObj - >> jobs. Whenever an API wants to work with a domain object [1], it shall >> acquire a job. This job then prevents other threads from modifying the >> object meanwhile The threads wanting to work over the same object will >> serialize. The whole job control is described in src/qemu/THREADS.txt so >> I would start there. >> >> Michal >> >> 1: actually, that is not entirely true. Acquiring a job is required only >> for those APIs which want to unlock the domain object in the middle. >> Therefore, some 'short' APIs have no job acquiring implemented (e.g. >> qemuDomainGetInfo()). But some other 'short' APIs do (e.g. >> qemuDomainReset()). >> > > What if you have a function which changes something, or even looks up > something > and it does not need to communicate with QEMU at all, but it is called > when some > other async job is waiting for a monitor. I know we discussed this > multiple > times and I always forget some minor detail, so forgive me if I'm asking > this > for 27th time already. But shouldn't that function also have a job? Oh yeah, I wasn't trying to be exhaustive and cover all corner cases when introducing jobs to somebody who hears about the concept for the first time. If a function modifies internal state then it should grab _MODIFY job. This is, as you correctly say, to mutually exclude with another thread that is doing some other modification. But frankly, I don't remember all the details. I recall discussing whether each API should grab a job, but I just don't remember the details. > Another > question is if this should also be the case for just reading and > returning some > info about the domain, shouldn't it also take a job, even if it is just a > QEMU_JOB_QUERY? Otherwise we'd have to make sure that before each > unlock of a > domain (even during a job) all the data integrity is kept. I'm not completely sure what you are asking here. And how it relates to storage driver. Michal