[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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 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.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]