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@xxxxxxxxxx>
> ---
> 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 */
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
