Re: [GSoC][PATCH v2 3/6] qemu_domainjob: `maxQueuedJobs` added to `qemuDomainJobPrivate`
On 8/18/20 10:48 AM, Erik Skultety wrote:
On Mon, Aug 17, 2020 at 10:37:18AM +0530, Prathamesh Chavan wrote:
> Reference to `maxQueuedJobs` required us to access
> config of the qemu-driver. And creating its copy in
> the `qemuDomainJob` helped us access the variable
> without referencing the driver's config.
>
> Signed-off-by: Prathamesh Chavan <pc44800(a)gmail.com>
> ---
> src/qemu/qemu_domain.c | 5 ++++-
> src/qemu/qemu_domainjob.c | 13 +++++++------
> src/qemu/qemu_domainjob.h | 4 +++-
> 3 files changed, 14 insertions(+), 8 deletions(-)
>
Looking at this again, I don't think we want to move any of the
jobs_queued or
maxQueuedJobs attributes to the job structure, I'm sorry I didn't spot this
right away in the previous revision of the series, I take responsibility for
it.
The job structure represents a single job, so from the design POV,
referencing both jobs_queued (which is really tied to a domain) and
maxQueuedJobs (which is a global driver setting) from within the job structure
simply doesn't feel right. Instead, I think qemuDomainObjBeginJobInternal will
simply have to be split in parts specific to QEMU that have access to the
driver and hypervisor-agnostic bits that can be moved to src/hypervisor.
Actually, I think maxQueuedJobs is job specific and not a global driver
setting. I mean, I can imagine other drivers benefiting from it too.
It's true that we currently don't allow specifying different values for
different domains and only apply the setting from qemu.conf, but in the
future we might make this available in domain XML somehow [1] and thus
be per domain. Moreover, I can imagine users wanting to set the limit
for volumes too [2].
Michal
1: this refers to discussion we had on the list (too lazy to find the
link, sorry) about the qemu:///embed. So far, if an user wants to set
some values, they have to create qemu.conf in the root before connecting
to the embed driver (so that the driver loads them). The idea was to
allow them to specify everything in domain XML so no file needs to be
created.
2: although, it's not as bad as it used to be, but with rotational disks
having two processes accessing different portions of a file resulted in
very poor performance as the head had to jump up & down.