On 06/08/2018 09:45 AM, Michal Privoznik wrote:
This enum will list all possible jobs for guest agent. The idea is when a thread needs to talk to guest agent only it will take the QEMU_AGENT_JOB instead of QEMU_JOB helping better concurrency.
Consider: Introduce guest agent specific job categories to allow threads to run agent monitor specific jobs while normal monitor jobs can also be running. Alter _qemuDomainJobObj in order to duplicate certain fields that will be used for guest agent specific tasks to increase concurrency and throughput and reduce serialization.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_domain.c | 4 ++++ src/qemu/qemu_domain.h | 15 +++++++++++++++ 2 files changed, 19 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 11c261db1a..b8e34c1c2c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -93,6 +93,10 @@ VIR_ENUM_IMPL(qemuDomainJob, QEMU_JOB_LAST, "async nested", );
+VIR_ENUM_IMPL(qemuDomainAgentJob, QEMU_AGENT_JOB_LAST, + "none" +); +
I think merging patch3 in here is perfectly reasonable, unless of course you're looking to get your patch counts up to stay ahead of Peter ;-)
VIR_ENUM_IMPL(qemuDomainAsyncJob, QEMU_ASYNC_JOB_LAST, "none", "migration out", diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index f17157b951..709b42e6fd 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -82,6 +82,13 @@ typedef enum { } qemuDomainJob; VIR_ENUM_DECL(qemuDomainJob)
+typedef enum { + QEMU_AGENT_JOB_NONE = 0, /* No agent job. */ + + QEMU_AGENT_JOB_LAST +} qemuDomainAgentJob; +VIR_ENUM_DECL(qemuDomainAgentJob) + /* Async job consists of a series of jobs that may change state. Independent * jobs that do not change state (and possibly others if explicitly allowed by * current async job) are allowed to be run even if async job is active. @@ -158,11 +165,19 @@ typedef struct _qemuDomainJobObj qemuDomainJobObj; typedef qemuDomainJobObj *qemuDomainJobObjPtr; struct _qemuDomainJobObj { virCond cond; /* Use to coordinate jobs */ + + /* The following members are for QEMU_JOB_* */ qemuDomainJob active; /* Currently running job */ unsigned long long owner; /* Thread id which set current job */ const char *ownerAPI; /* The API which owns the job */ unsigned long long started; /* When the current job started */
+ /* The following members are for QEMU_AGENT_JOB_* */ + qemuDomainAgentJob agentActive; /* Currently running agent job */
Hmm... I seem to recall a recent patch that spoke against using an enum type for struct fields: https://www.redhat.com/archives/libvir-list/2018-June/msg00486.html While I know you're just copying the existing qemuDomainJob - this is the kind of stuff that just gets copied around and used elsewhere.
+ unsigned long long agentOwner; /* Thread id which set current agent job */ + const char *agentOwnerAPI; /* The API which owns the agent job */ + unsigned long long agentStarted; /* When the current agent job started */ +
FWIW: From the description in the next patch, I would think there would need to be an agentCond variable too. But reading patch 4 I know that's not quite the case. I have concerns over the combined @cond usage, but those are only towards some "future change" that misses some subtlety. And the following few fields are for async jobs - may as well add a comment here too... John
virCond asyncCond; /* Use to coordinate with async jobs */ qemuDomainAsyncJob asyncJob; /* Currently active async job */ unsigned long long asyncOwner; /* Thread which set current async job */