Re: [PATCH 1/9] qapi: New special feature flag "unstable"

[Markus Armbruster]

John Snow  writes:

> On Mon, Oct 25, 2021 at 1:26 AM Markus Armbruster  wrote:
>
>> By convention, names starting with "x-" are experimental.  The parts
>> of external interfaces so named may be withdrawn or changed
>> incompatibly in future releases.
>>
>> Drawback: promoting something from experimental to stable involves a
>> name change.  Client code needs to be updated.
>>
>> Moreover, the convention is not universally observed:
>>
>> * QOM type "input-barrier" has properties "x-origin", "y-origin".
>>   Looks accidental, but it's ABI since 4.2.
>>
>> * QOM types "memory-backend-file", "memory-backend-memfd",
>>   "memory-backend-ram", and "memory-backend-epc" have a property
>>   "x-use-canonical-path-for-ramblock-id" that is documented to be
>>   stable despite its name.
>>
>> We could document these exceptions, but documentation helps only
>> humans.  We want to recognize "unstable" in code, like "deprecated".
>>
>> Replace the convention by a new special feature flag "unstable".  It
>> will be recognized by the QAPI generator, like the existing feature
>> flag "deprecated", and unlike regular feature flags.
>>
>> This commit updates documentation and prepares tests.  The next commit
>> updates the QAPI schema.  The remaining patches update the QAPI
>> generator and wire up -compat policy checking.
>>
>> Signed-off-by: Markus Armbruster 

[...]

> Feels odd to combine the doc update *and* test prep, but eh, whatever.

I admit this is what was left after patch splitting and reshuffling.

> Reviewed-by: John Snow 

Thanks!

Re: [libvirt PATCH] node_device: Fix memory leak in udevProcessMediatedDevice

[Laine Stump]

On 10/25/21 11:32 AM, Jiri Denemark wrote:

One of the paths returned -1 directly without going through the cleanup
section.

Signed-off-by: Jiri Denemark 


Reviewed-by: Laine Stump 

Re: [PATCH] cosmetic: remove unused function return value

[Laine Stump]

On 10/24/21 1:40 AM, Ani Sinha wrote:

qemuBuildPMPCIRootHotplugCommandLine() returns 0 unconditionally. There is no
failure scenario at present. So clean up the code by removing integer return
from the function and also remove the failure check conditional from the
function call.
Also fix indentation for the above function call while at it.

Signed-off-by: Ani Sinha 


Reviewed-by: Laine Stump 

and pushed.


---
  src/qemu/qemu_command.c | 9 -
  1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d6df50ec73..093d8ae62c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3177,9 +3177,9 @@ qemuBuildSkipController(const virDomainControllerDef 
*controller,
  return false;
  }
  
-static int

+static void
  qemuBuildPMPCIRootHotplugCommandLine(virCommand *cmd,
-  const virDomainControllerDef *controller)
+ const virDomainControllerDef *controller)
  {
  if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
  controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT &&
@@ -3189,7 +3189,7 @@ qemuBuildPMPCIRootHotplugCommandLine(virCommand *cmd,
  virCommandAddArgFormat(cmd, "PIIX4_PM.acpi-root-pci-hotplug=%s",
 
virTristateSwitchTypeToString(controller->opts.pciopts.hotplug));
  }
-return 0;
+return;
  }
  
  static int

@@ -3207,8 +3207,7 @@ qemuBuildControllersByTypeCommandLine(virCommand *cmd,
  if (cont->type != type)
  continue;
  
-if (qemuBuildPMPCIRootHotplugCommandLine(cmd, cont))

-continue;
+qemuBuildPMPCIRootHotplugCommandLine(cmd, cont);
  
  if (qemuBuildSkipController(cont, def))

  continue;

Re: [PATCH] i440fx/hotplug: Fix error message format to conform to spec

[Laine Stump]

On 10/20/21 11:30 PM, Ani Sinha wrote:

Error messages must conform to spec as specified here:
https://www.libvirt.org/coding-style.html#error-message-format

This change makes some error messages conform to the spec above.

Fixes: 8eadf82fb5 ("conf: introduce option to enable/disable pci hotplug on pci-root 
controller")

Signed-off-by: Ani Sinha 


Reviewed-by: Laine Stump 

and pushed.


---
  src/qemu/qemu_validate.c| 6 +++---
  .../pc-i440fx-acpi-root-hotplug-disable.x86_64-5.1.0.err| 2 +-
  .../pc-i440fx-acpi-root-hotplug-enable.x86_64-5.1.0.err | 2 +-
  3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 3045e4b64b..f93b697265 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -3947,7 +3947,7 @@ qemuValidateDomainDeviceDefControllerPCI(const 
virDomainControllerDef *cont,
  case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
  if (!virQEMUCapsGet(qemuCaps, 
QEMU_CAPS_PIIX4_ACPI_ROOT_PCI_HOTPLUG)) {
  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("setting the %s property on a '%s' device is not 
supported by this QEMU binary"),
+   _("setting the '%s' property on a '%s' device is not 
supported by this QEMU binary"),
 "hotplug", "pci-root");
  return -1;
  }
@@ -3956,8 +3956,8 @@ qemuValidateDomainDeviceDefControllerPCI(const 
virDomainControllerDef *cont,
  case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
  if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCIE_ROOT_PORT_HOTPLUG)) {
  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("setting the hotplug property on a '%s' device is 
not supported by this QEMU binary"),
-   modelName);
+   _("setting the '%s' property on a '%s' device is not 
supported by this QEMU binary"),
+   "hotplug", modelName);
  return -1;
  }
  break;
diff --git 
a/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-disable.x86_64-5.1.0.err 
b/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-disable.x86_64-5.1.0.err
index b507f1f8bc..55ec41c476 100644
--- 
a/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-disable.x86_64-5.1.0.err
+++ 
b/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-disable.x86_64-5.1.0.err
@@ -1 +1 @@
-unsupported configuration: setting the hotplug property on a 'pci-root' device 
is not supported by this QEMU binary
+unsupported configuration: setting the 'hotplug' property on a 'pci-root' 
device is not supported by this QEMU binary
diff --git 
a/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-enable.x86_64-5.1.0.err 
b/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-enable.x86_64-5.1.0.err
index b507f1f8bc..55ec41c476 100644
--- a/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-enable.x86_64-5.1.0.err
+++ b/tests/qemuxml2argvdata/pc-i440fx-acpi-root-hotplug-enable.x86_64-5.1.0.err
@@ -1 +1 @@
-unsupported configuration: setting the hotplug property on a 'pci-root' device 
is not supported by this QEMU binary
+unsupported configuration: setting the 'hotplug' property on a 'pci-root' 
device is not supported by this QEMU binary

Re: [PATCH 8/9] qapi: Factor out compat_policy_input_ok()

[John Snow]

On Mon, Oct 25, 2021 at 1:25 AM Markus Armbruster  wrote:

> The code to check policy for handling deprecated input is triplicated.
> Factor it out into compat_policy_input_ok() before I mess with it in
> the next commit.
>
> Signed-off-by: Markus Armbruster 
>


(Skipping C-only patches for quick review. I'll trust you on these.)

--js

Re: [PATCH 9/9] qapi: Extend -compat to set policy for unstable interfaces

[John Snow]

On Mon, Oct 25, 2021 at 1:26 AM Markus Armbruster  wrote:

> New option parameters unstable-input and unstable-output set policy
> for unstable interfaces just like deprecated-input and
> deprecated-output set policy for deprecated interfaces (see commit
> 6dd75472d5 "qemu-options: New -compat to set policy for deprecated
> interfaces").  This is intended for testing users of the management
> interfaces.  It is experimental.
>
> For now, this covers only syntactic aspects of QMP, i.e. stuff tagged
> with feature 'unstable'.  We may want to extend it to cover semantic
> aspects, or the command line.
>
> Note that there is no good way for management application to detect
> presence of these new option parameters: they are not visible output
> of query-qmp-schema or query-command-line-options.  Tolerable, because
> it's meant for testing.  If running with -compat fails, skip the test.
>
> Signed-off-by: Markus Armbruster 
> ---
>  qapi/compat.json  |  6 +-
>  include/qapi/util.h   |  1 +
>  qapi/qmp-dispatch.c   |  6 ++
>  qapi/qobject-output-visitor.c |  8 ++--
>  qemu-options.hx   | 20 +++-
>  scripts/qapi/events.py| 10 ++
>  scripts/qapi/schema.py| 10 ++
>  7 files changed, 49 insertions(+), 12 deletions(-)
>
> diff --git a/qapi/compat.json b/qapi/compat.json
> index 74a8493d3d..9bc9804abb 100644
> --- a/qapi/compat.json
> +++ b/qapi/compat.json
> @@ -47,9 +47,13 @@
>  #
>  # @deprecated-input: how to handle deprecated input (default 'accept')
>  # @deprecated-output: how to handle deprecated output (default 'accept')
> +# @unstable-input: how to handle unstable input (default 'accept')
> +# @unstable-output: how to handle unstable output (default 'accept')
>  #
>  # Since: 6.0
>  ##
>  { 'struct': 'CompatPolicy',
>'data': { '*deprecated-input': 'CompatPolicyInput',
> -'*deprecated-output': 'CompatPolicyOutput' } }
> +'*deprecated-output': 'CompatPolicyOutput',
> +'*unstable-input': 'CompatPolicyInput',
> +'*unstable-output': 'CompatPolicyOutput' } }
> diff --git a/include/qapi/util.h b/include/qapi/util.h
> index 0cc98db9f9..81a2b13a33 100644
> --- a/include/qapi/util.h
> +++ b/include/qapi/util.h
> @@ -13,6 +13,7 @@
>
>  typedef enum {
>  QAPI_DEPRECATED,
> +QAPI_UNSTABLE,
>  } QapiSpecialFeature;
>
>  typedef struct QEnumLookup {
> diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
> index e29ade134c..c5c6e521a2 100644
> --- a/qapi/qmp-dispatch.c
> +++ b/qapi/qmp-dispatch.c
> @@ -59,6 +59,12 @@ bool compat_policy_input_ok(unsigned special_features,
>  error_class, kind, name, errp)) {
>  return false;
>  }
> +if ((special_features & (1u << QAPI_UNSTABLE))
> +&& !compat_policy_input_ok1("Unstable",
> +policy->unstable_input,
> +error_class, kind, name, errp)) {
> +return false;
> +}
>  return true;
>  }
>
> diff --git a/qapi/qobject-output-visitor.c b/qapi/qobject-output-visitor.c
> index b5c6564cbb..74770edd73 100644
> --- a/qapi/qobject-output-visitor.c
> +++ b/qapi/qobject-output-visitor.c
> @@ -212,8 +212,12 @@ static bool qobject_output_type_null(Visitor *v,
> const char *name,
>  static bool qobject_output_policy_skip(Visitor *v, const char *name,
> unsigned special_features)
>  {
> -return !(special_features && 1u << QAPI_DEPRECATED)
> -|| v->compat_policy.deprecated_output ==
> COMPAT_POLICY_OUTPUT_HIDE;
> +CompatPolicy *pol = >compat_policy;
> +
> +return ((special_features & 1u << QAPI_DEPRECATED)
> +&& pol->deprecated_output == COMPAT_POLICY_OUTPUT_HIDE)
> +|| ((special_features & 1u << QAPI_UNSTABLE)
> +&& pol->unstable_output == COMPAT_POLICY_OUTPUT_HIDE);
>  }
>
>  /* Finish building, and return the root object.
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 5f375bbfa6..f051536b63 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3641,7 +3641,9 @@ DEFHEADING(Debug/Expert options:)
>
>  DEF("compat", HAS_ARG, QEMU_OPTION_compat,
>  "-compat
> [deprecated-input=accept|reject|crash][,deprecated-output=accept|hide]\n"
> -"Policy for handling deprecated management
> interfaces\n",
> +"Policy for handling deprecated management
> interfaces\n"
> +"-compat
> [unstable-input=accept|reject|crash][,unstable-output=accept|hide]\n"
> +"Policy for handling unstable management
> interfaces\n",
>  QEMU_ARCH_ALL)
>  SRST
>  ``-compat
> [deprecated-input=@var{input-policy}][,deprecated-output=@var{output-policy}]``
> @@ -3659,6 +3661,22 @@ SRST
>  Suppress deprecated command results and events
>
>  Limitation: covers only syntactic aspects of QMP.
> +
> +``-compat
> 

Re: [PATCH 7/9] qapi: Generalize enum member policy checking

[John Snow]

On Mon, Oct 25, 2021 at 1:26 AM Markus Armbruster  wrote:

> The code to check enumeration value policy can see special feature
> flag 'deprecated' in QEnumLookup member flags[value].  I want to make
> feature flag 'unstable' visible there as well, so I can add policy for
> it.
>
> Instead of extending flags[], replace it by @special_features (a
> bitset of QapiSpecialFeature), because that's how special features get
> passed around elsewhere.
>
> Signed-off-by: Markus Armbruster 
> ---
>  include/qapi/util.h|  5 +
>  qapi/qapi-visit-core.c |  3 ++-
>  scripts/qapi/types.py  | 22 --
>  3 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/include/qapi/util.h b/include/qapi/util.h
> index 7a8d5c7d72..0cc98db9f9 100644
> --- a/include/qapi/util.h
> +++ b/include/qapi/util.h
> @@ -15,12 +15,9 @@ typedef enum {
>  QAPI_DEPRECATED,
>  } QapiSpecialFeature;
>
> -/* QEnumLookup flags */
> -#define QAPI_ENUM_DEPRECATED 1
> -
>  typedef struct QEnumLookup {
>  const char *const *array;
> -const unsigned char *const flags;
> +const unsigned char *const special_features;
>  const int size;
>  } QEnumLookup;
>
> diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> index b4a81f1757..5572d90efb 100644
> --- a/qapi/qapi-visit-core.c
> +++ b/qapi/qapi-visit-core.c
> @@ -407,7 +407,8 @@ static bool input_type_enum(Visitor *v, const char
> *name, int *obj,
>  return false;
>  }
>
> -if (lookup->flags && (lookup->flags[value] & QAPI_ENUM_DEPRECATED)) {
> +if (lookup->special_features
> +&& (lookup->special_features[value] & QAPI_DEPRECATED)) {
>  switch (v->compat_policy.deprecated_input) {
>  case COMPAT_POLICY_INPUT_ACCEPT:
>  break;
> diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
> index ab2441adc9..3013329c24 100644
> --- a/scripts/qapi/types.py
> +++ b/scripts/qapi/types.py
> @@ -16,7 +16,7 @@
>  from typing import List, Optional
>
>  from .common import c_enum_const, c_name, mcgen
> -from .gen import QAPISchemaModularCVisitor, ifcontext
> +from .gen import QAPISchemaModularCVisitor, gen_special_features,
> ifcontext
>  from .schema import (
>  QAPISchema,
>  QAPISchemaEnumMember,
> @@ -39,7 +39,7 @@ def gen_enum_lookup(name: str,
>  members: List[QAPISchemaEnumMember],
>  prefix: Optional[str] = None) -> str:
>  max_index = c_enum_const(name, '_MAX', prefix)
> -flags = ''
> +feats = ''
>  ret = mcgen('''
>
>  const QEnumLookup %(c_name)s_lookup = {
> @@ -54,19 +54,21 @@ def gen_enum_lookup(name: str,
>  ''',
>   index=index, name=memb.name)
>  ret += memb.ifcond.gen_endif()
> -if 'deprecated' in (f.name for f in memb.features):
> -flags += mcgen('''
> -[%(index)s] = QAPI_ENUM_DEPRECATED,
> -''',
> -   index=index)
>
> -if flags:
> +special_features = gen_special_features(memb.features)
> +if special_features != '0':
>

Though, I have to admit the common reoccurrence of this pattern suggests to
me that gen_special_features really ought to be returning something false-y
in these cases. Something about testing for the empty case with something
that represents, but isn't empty, gives me a brief pause.

Not willing to wage war over it.


> +feats += mcgen('''
> +[%(index)s] = %(special_features)s,
> +''',
> +   index=index, special_features=special_features)
> +
> +if feats:
>  ret += mcgen('''
>  },
> -.flags = (const unsigned char[%(max_index)s]) {
> +.special_features = (const unsigned char[%(max_index)s]) {
>  ''',
>   max_index=max_index)
> -ret += flags
> +ret += feats
>
>  ret += mcgen('''
>  },
> --
> 2.31.1
>
>
Python bits: Acked-by: John Snow 

Re: [PATCH 6/9] qapi: Generalize command policy checking

[John Snow]

On Mon, Oct 25, 2021 at 1:25 AM Markus Armbruster  wrote:

> The code to check command policy can see special feature flag
> 'deprecated' as command flag QCO_DEPRECATED.  I want to make feature
> flag 'unstable' visible there as well, so I can add policy for it.
>
> To let me make it visible, add member @special_features (a bitset of
> QapiSpecialFeature) to QmpCommand, and adjust the generator to pass it
> through qmp_register_command().  Then replace "QCO_DEPRECATED in
> @flags" by QAPI_DEPRECATED in @special_features", and drop
> QCO_DEPRECATED.
>
> Signed-off-by: Markus Armbruster 
> ---
>  include/qapi/qmp/dispatch.h  | 5 +++--
>  monitor/misc.c   | 6 --
>  qapi/qmp-dispatch.c  | 2 +-
>  qapi/qmp-registry.c  | 4 +++-
>  storage-daemon/qemu-storage-daemon.c | 3 ++-
>  scripts/qapi/commands.py | 9 -
>  6 files changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
> index 0ce88200b9..1e4240fd0d 100644
> --- a/include/qapi/qmp/dispatch.h
> +++ b/include/qapi/qmp/dispatch.h
> @@ -25,7 +25,6 @@ typedef enum QmpCommandOptions
>  QCO_ALLOW_OOB =  (1U << 1),
>  QCO_ALLOW_PRECONFIG   =  (1U << 2),
>  QCO_COROUTINE =  (1U << 3),
> -QCO_DEPRECATED=  (1U << 4),
>  } QmpCommandOptions;
>
>  typedef struct QmpCommand
> @@ -34,6 +33,7 @@ typedef struct QmpCommand
>  /* Runs in coroutine context if QCO_COROUTINE is set */
>  QmpCommandFunc *fn;
>  QmpCommandOptions options;
> +unsigned special_features;
>  QTAILQ_ENTRY(QmpCommand) node;
>  bool enabled;
>  const char *disable_reason;
> @@ -42,7 +42,8 @@ typedef struct QmpCommand
>  typedef QTAILQ_HEAD(QmpCommandList, QmpCommand) QmpCommandList;
>
>  void qmp_register_command(QmpCommandList *cmds, const char *name,
> -  QmpCommandFunc *fn, QmpCommandOptions options);
> +  QmpCommandFunc *fn, QmpCommandOptions options,
> +  unsigned special_features);
>  const QmpCommand *qmp_find_command(const QmpCommandList *cmds,
> const char *name);
>  void qmp_disable_command(QmpCommandList *cmds, const char *name,
> diff --git a/monitor/misc.c b/monitor/misc.c
> index 3556b177f6..c2d227a07c 100644
> --- a/monitor/misc.c
> +++ b/monitor/misc.c
> @@ -230,11 +230,13 @@ static void monitor_init_qmp_commands(void)
>
>  qmp_init_marshal(_commands);
>
> -qmp_register_command(_commands, "device_add", qmp_device_add, 0);
> +qmp_register_command(_commands, "device_add",
> + qmp_device_add, 0, 0);
>
>  QTAILQ_INIT(_cap_negotiation_commands);
>  qmp_register_command(_cap_negotiation_commands,
> "qmp_capabilities",
> - qmp_marshal_qmp_capabilities,
> QCO_ALLOW_PRECONFIG);
> + qmp_marshal_qmp_capabilities,
> + QCO_ALLOW_PRECONFIG, 0);
>  }
>
>  /* Set the current CPU defined by the user. Callers must hold BQL. */
> diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
> index 7e943a0af5..8cca18c891 100644
> --- a/qapi/qmp-dispatch.c
> +++ b/qapi/qmp-dispatch.c
> @@ -176,7 +176,7 @@ QDict *qmp_dispatch(const QmpCommandList *cmds,
> QObject *request,
>"The command %s has not been found", command);
>  goto out;
>  }
> -if (cmd->options & QCO_DEPRECATED) {
> +if (cmd->special_features & 1u << QAPI_DEPRECATED) {
>  switch (compat_policy.deprecated_input) {
>  case COMPAT_POLICY_INPUT_ACCEPT:
>  break;
> diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c
> index f78c064aae..485bc5e6fc 100644
> --- a/qapi/qmp-registry.c
> +++ b/qapi/qmp-registry.c
> @@ -16,7 +16,8 @@
>  #include "qapi/qmp/dispatch.h"
>
>  void qmp_register_command(QmpCommandList *cmds, const char *name,
> -  QmpCommandFunc *fn, QmpCommandOptions options)
> +  QmpCommandFunc *fn, QmpCommandOptions options,
> +  unsigned special_features)
>  {
>  QmpCommand *cmd = g_malloc0(sizeof(*cmd));
>
> @@ -27,6 +28,7 @@ void qmp_register_command(QmpCommandList *cmds, const
> char *name,
>  cmd->fn = fn;
>  cmd->enabled = true;
>  cmd->options = options;
> +cmd->special_features = special_features;
>  QTAILQ_INSERT_TAIL(cmds, cmd, node);
>  }
>
> diff --git a/storage-daemon/qemu-storage-daemon.c
> b/storage-daemon/qemu-storage-daemon.c
> index 10a1a33761..52cf17e8ac 100644
> --- a/storage-daemon/qemu-storage-daemon.c
> +++ b/storage-daemon/qemu-storage-daemon.c
> @@ -146,7 +146,8 @@ static void init_qmp_commands(void)
>
>  QTAILQ_INIT(_cap_negotiation_commands);
>  qmp_register_command(_cap_negotiation_commands,
> "qmp_capabilities",
> - qmp_marshal_qmp_capabilities,
> QCO_ALLOW_PRECONFIG);
> + 

Re: [PATCH 5/9] qapi: Generalize struct member policy checking

[John Snow]

On Mon, Oct 25, 2021 at 1:25 AM Markus Armbruster  wrote:

> The generated visitor functions call visit_deprecated_accept() and
> visit_deprecated() when visiting a struct member with special feature
> flag 'deprecated'.  This makes the feature flag visible to the actual
> visitors.  I want to make feature flag 'unstable' visible there as
> well, so I can add policy for it.
>
> To let me make it visible, replace these functions by
> visit_policy_reject() and visit_policy_skip(), which take the member's
> special features as an argument.  Note that the new functions have the
> opposite sense, i.e. the return value flips.
>
> Signed-off-by: Markus Armbruster 
> ---
>  include/qapi/visitor-impl.h   |  6 --
>  include/qapi/visitor.h| 17 +
>  qapi/qapi-forward-visitor.c   | 16 +---
>  qapi/qapi-visit-core.c| 22 --
>  qapi/qobject-input-visitor.c  | 15 ++-
>  qapi/qobject-output-visitor.c |  9 ++---
>  qapi/trace-events |  4 ++--
>  scripts/qapi/visit.py | 14 +++---
>  8 files changed, 63 insertions(+), 40 deletions(-)
>
> diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
> index 72b6537bef..2badec5ba4 100644
> --- a/include/qapi/visitor-impl.h
> +++ b/include/qapi/visitor-impl.h
> @@ -114,10 +114,12 @@ struct Visitor
>  void (*optional)(Visitor *v, const char *name, bool *present);
>
>  /* Optional */
> -bool (*deprecated_accept)(Visitor *v, const char *name, Error **errp);
> +bool (*policy_reject)(Visitor *v, const char *name,
> +  unsigned special_features, Error **errp);
>
>  /* Optional */
> -bool (*deprecated)(Visitor *v, const char *name);
> +bool (*policy_skip)(Visitor *v, const char *name,
> +unsigned special_features);
>
>  /* Must be set */
>  VisitorType type;
> diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
> index dcb96018a9..d53a84c9ba 100644
> --- a/include/qapi/visitor.h
> +++ b/include/qapi/visitor.h
> @@ -461,22 +461,31 @@ void visit_end_alternate(Visitor *v, void **obj);
>  bool visit_optional(Visitor *v, const char *name, bool *present);
>
>  /*
> - * Should we reject deprecated member @name?
> + * Should we reject member @name due to policy?
> + *
> + * @special_features is the member's special features encoded as a
> + * bitset of QapiSpecialFeature.
>   *
>   * @name must not be NULL.  This function is only useful between
>   * visit_start_struct() and visit_end_struct(), since only objects
>   * have deprecated members.
>   */
> -bool visit_deprecated_accept(Visitor *v, const char *name, Error **errp);
> +bool visit_policy_reject(Visitor *v, const char *name,
> + unsigned special_features, Error **errp);
>
>  /*
> - * Should we visit deprecated member @name?
> + *
> + * Should we skip member @name due to policy?
> + *
> + * @special_features is the member's special features encoded as a
> + * bitset of QapiSpecialFeature.
>   *
>   * @name must not be NULL.  This function is only useful between
>   * visit_start_struct() and visit_end_struct(), since only objects
>   * have deprecated members.
>   */
> -bool visit_deprecated(Visitor *v, const char *name);
> +bool visit_policy_skip(Visitor *v, const char *name,
> +   unsigned special_features);
>
>  /*
>   * Set policy for handling deprecated management interfaces.
> diff --git a/qapi/qapi-forward-visitor.c b/qapi/qapi-forward-visitor.c
> index a4b111e22a..25d098aa8a 100644
> --- a/qapi/qapi-forward-visitor.c
> +++ b/qapi/qapi-forward-visitor.c
> @@ -246,25 +246,27 @@ static void forward_field_optional(Visitor *v, const
> char *name, bool *present)
>  visit_optional(ffv->target, name, present);
>  }
>
> -static bool forward_field_deprecated_accept(Visitor *v, const char *name,
> -Error **errp)
> +static bool forward_field_policy_reject(Visitor *v, const char *name,
> +unsigned special_features,
> +Error **errp)
>  {
>  ForwardFieldVisitor *ffv = to_ffv(v);
>
>  if (!forward_field_translate_name(ffv, , errp)) {
>  return false;
>  }
> -return visit_deprecated_accept(ffv->target, name, errp);
> +return visit_policy_reject(ffv->target, name, special_features, errp);
>  }
>
> -static bool forward_field_deprecated(Visitor *v, const char *name)
> +static bool forward_field_policy_skip(Visitor *v, const char *name,
> +  unsigned special_features)
>  {
>  ForwardFieldVisitor *ffv = to_ffv(v);
>
>  if (!forward_field_translate_name(ffv, , NULL)) {
>  return false;
>  }
> -return visit_deprecated(ffv->target, name);
> +return visit_policy_skip(ffv->target, name, special_features);
>  }
>
>  static void forward_field_complete(Visitor *v, void *opaque)
> @@ -313,8 

Re: [PATCH 4/9] qapi: Tools for sets of special feature flags in generated code

[John Snow]

On Mon, Oct 25, 2021 at 1:25 AM Markus Armbruster  wrote:

> New enum QapiSpecialFeature enumerates the special feature flags.
>
> New helper gen_special_features() returns code to represent a
> collection of special feature flags as a bitset.
>
> The next few commits will put them to use.
>
> Signed-off-by: Markus Armbruster 
> ---
>  include/qapi/util.h|  4 
>  scripts/qapi/gen.py| 13 +
>  scripts/qapi/schema.py |  3 +++
>  3 files changed, 20 insertions(+)
>
> diff --git a/include/qapi/util.h b/include/qapi/util.h
> index 257c600f99..7a8d5c7d72 100644
> --- a/include/qapi/util.h
> +++ b/include/qapi/util.h
> @@ -11,6 +11,10 @@
>  #ifndef QAPI_UTIL_H
>  #define QAPI_UTIL_H
>
> +typedef enum {
> +QAPI_DEPRECATED,
> +} QapiSpecialFeature;
> +
>  /* QEnumLookup flags */
>  #define QAPI_ENUM_DEPRECATED 1
>
> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
> index 2ec1e7b3b6..9d07b88cf6 100644
> --- a/scripts/qapi/gen.py
> +++ b/scripts/qapi/gen.py
> @@ -29,6 +29,7 @@
>  mcgen,
>  )
>  from .schema import (
> +QAPISchemaFeature,
>  QAPISchemaIfCond,
>  QAPISchemaModule,
>  QAPISchemaObjectType,
> @@ -37,6 +38,18 @@
>  from .source import QAPISourceInfo
>
>
> +def gen_special_features(features: QAPISchemaFeature):
> +ret = ''
> +sep = ''
> +
> +for feat in features:
> +if feat.is_special():
> +ret += ('%s1u << QAPI_%s' % (sep, feat.name.upper()))
>

Building the constant name here "feels" fragile, but I'll trust that the
test suite and/or the compiler will catch us if we accidentally goof up
this mapping. In the interest of simplicity, then, "sure, why not."


> +sep = ' | '
> +
>
+return ret or '0'
> +
>

Subjectively more pythonic:

special_features = [f"1u << QAPI_{feat.name.upper()}" for feat in features
if feat.is_special()]
ret = ' | '.join(special_features)
return ret or '0'

A bit more dense, but more functional. Up to you, but I find join() easier
to read and reason about for the presence of separators.


> +
>  class QAPIGen:
>  def __init__(self, fname: str):
>  self.fname = fname
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index 6d5f46509a..55f82d7389 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -725,6 +725,9 @@ def connect_doc(self, doc):
>  class QAPISchemaFeature(QAPISchemaMember):
>  role = 'feature'
>
> +def is_special(self):
> +return self.name in ('deprecated')
> +
>

alrighty.

(Briefly wondered: is it worth naming special features as a property of the
class, but with only two names, it's probably fine enough to leave it
embedded in the method logic. Only a style thing and doesn't have any
actual impact that I can imagine, so ... nevermind.)


>
>  class QAPISchemaObjectTypeMember(QAPISchemaMember):
>  def __init__(self, name, info, typ, optional, ifcond=None,
> features=None):
> --
> 2.31.1
>
>
Well, either way:

Reviewed-by: John Snow 

Re: [PATCH 3/9] qapi: Eliminate QCO_NO_OPTIONS for a slight simplification

[John Snow]

On Mon, Oct 25, 2021 at 1:25 AM Markus Armbruster  wrote:

> Signed-off-by: Markus Armbruster 
> ---
>  include/qapi/qmp/dispatch.h | 1 -
>  monitor/misc.c  | 3 +--
>  scripts/qapi/commands.py| 5 +
>  3 files changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
> index 075203dc67..0ce88200b9 100644
> --- a/include/qapi/qmp/dispatch.h
> +++ b/include/qapi/qmp/dispatch.h
> @@ -21,7 +21,6 @@ typedef void (QmpCommandFunc)(QDict *, QObject **, Error
> **);
>
>  typedef enum QmpCommandOptions
>  {
> -QCO_NO_OPTIONS=  0x0,
>  QCO_NO_SUCCESS_RESP   =  (1U << 0),
>  QCO_ALLOW_OOB =  (1U << 1),
>  QCO_ALLOW_PRECONFIG   =  (1U << 2),
> diff --git a/monitor/misc.c b/monitor/misc.c
> index ffe7966870..3556b177f6 100644
> --- a/monitor/misc.c
> +++ b/monitor/misc.c
> @@ -230,8 +230,7 @@ static void monitor_init_qmp_commands(void)
>
>  qmp_init_marshal(_commands);
>
> -qmp_register_command(_commands, "device_add", qmp_device_add,
> - QCO_NO_OPTIONS);
> +qmp_register_command(_commands, "device_add", qmp_device_add, 0);
>
>  QTAILQ_INIT(_cap_negotiation_commands);
>  qmp_register_command(_cap_negotiation_commands,
> "qmp_capabilities",
> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
> index 3654825968..c8a975528f 100644
> --- a/scripts/qapi/commands.py
> +++ b/scripts/qapi/commands.py
> @@ -229,15 +229,12 @@ def gen_register_command(name: str,
>  if coroutine:
>  options += ['QCO_COROUTINE']
>
> -if not options:
> -options = ['QCO_NO_OPTIONS']
> -
>  ret = mcgen('''
>  qmp_register_command(cmds, "%(name)s",
>   qmp_marshal_%(c_name)s, %(opts)s);
>  ''',
>  name=name, c_name=c_name(name),
> -opts=" | ".join(options))
> +opts=' | '.join(options) or 0)
>  return ret
>
>
>
I'm not a big fan of naked constants on the C side, but the generator
simplification is nice. I suppose it's worth the trade-off if you like it
better this way.

"eh".

Reviewed-by: John Snow 

Re: [PATCH 2/9] qapi: Mark unstable QMP parts with feature 'unstable'

[John Snow]

On Mon, Oct 25, 2021 at 1:25 AM Markus Armbruster  wrote:

> Add special feature 'unstable' everywhere the name starts with 'x-',
> except for InputBarrierProperties member x-origin and
> MemoryBackendProperties member x-use-canonical-path-for-ramblock-id,
> because these two are actually stable.
>
> Signed-off-by: Markus Armbruster 
> ---
>  qapi/block-core.json | 123 +++
>  qapi/migration.json  |  35 +---
>  qapi/misc.json   |   6 ++-
>  qapi/qom.json|  11 ++--
>  4 files changed, 130 insertions(+), 45 deletions(-)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 6d3217abb6..ce2c1352cb 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1438,6 +1438,9 @@
>  #
>  # @x-perf: Performance options. (Since 6.0)
>  #
> +# Features:
> +# @unstable: Member @x-perf is experimental.
> +#
>

It'd be a lot cooler if we could annotate the unstable member directly
instead of confusing it with the syntax that might describe the entire
struct/union/command/etc, but ... eh, it's just a doc field, so I'm not
gonna press on this. I don't have the energy to get into a doc formatting
standard discussion right now, so: sure, why not?


>  # Note: @on-source-error and @on-target-error only affect background
>  #   I/O.  If an error occurs during a guest write request, the
> device's
>  #   rerror/werror actions will be used.
> @@ -1452,7 +1455,9 @@
>  '*on-source-error': 'BlockdevOnError',
>  '*on-target-error': 'BlockdevOnError',
>  '*auto-finalize': 'bool', '*auto-dismiss': 'bool',
> -'*filter-node-name': 'str', '*x-perf': 'BackupPerf'  } }
> +'*filter-node-name': 'str',
> +'*x-perf': { 'type': 'BackupPerf',
> + 'features': [ 'unstable' ] } } }
>
>  ##
>  # @DriveBackup:
> @@ -1916,9 +1921,13 @@
>  #
>  # Get the block graph.
>  #
> +# Features:
> +# @unstable: This command is meant for debugging.
> +#
>  # Since: 4.0
>  ##
> -{ 'command': 'x-debug-query-block-graph', 'returns': 'XDbgBlockGraph' }
> +{ 'command': 'x-debug-query-block-graph', 'returns': 'XDbgBlockGraph',
> +  'features': [ 'unstable' ] }
>
>  ##
>  # @drive-mirror:
> @@ -2257,6 +2266,9 @@
>  #
>  # Get bitmap SHA256.
>  #
> +# Features:
> +# @unstable: This command is meant for debugging.
> +#
>  # Returns: - BlockDirtyBitmapSha256 on success
>  #  - If @node is not a valid block device, DeviceNotFound
>  #  - If @name is not found or if hashing has failed, GenericError
> with an
> @@ -2265,7 +2277,8 @@
>  # Since: 2.10
>  ##
>  { 'command': 'x-debug-block-dirty-bitmap-sha256',
> -  'data': 'BlockDirtyBitmap', 'returns': 'BlockDirtyBitmapSha256' }
> +  'data': 'BlockDirtyBitmap', 'returns': 'BlockDirtyBitmapSha256',
> +  'features': [ 'unstable' ] }
>
>  ##
>  # @blockdev-mirror:
> @@ -2495,27 +2508,57 @@
>  #
>  # Properties for throttle-group objects.
>  #
> -# The options starting with x- are aliases for the same key without x- in
> -# the @limits object. As indicated by the x- prefix, this is not a stable
> -# interface and may be removed or changed incompatibly in the future. Use
> -# @limits for a supported stable interface.
> -#
>  # @limits: limits to apply for this throttle group
>  #
> +# Features:
> +# @unstable: All members starting with x- are aliases for the same key
> +#without x- in the @limits object.  This is not a stable
> +#interface and may be removed or changed incompatibly in
> +#the future.  Use @limits for a supported stable
> +#interface.
> +#
>  # Since: 2.11
>  ##
>  { 'struct': 'ThrottleGroupProperties',
>'data': { '*limits': 'ThrottleLimits',
> -'*x-iops-total' : 'int', '*x-iops-total-max' : 'int',
> -'*x-iops-total-max-length' : 'int', '*x-iops-read' : 'int',
> -'*x-iops-read-max' : 'int', '*x-iops-read-max-length' : 'int',
> -'*x-iops-write' : 'int', '*x-iops-write-max' : 'int',
> -'*x-iops-write-max-length' : 'int', '*x-bps-total' : 'int',
> -'*x-bps-total-max' : 'int', '*x-bps-total-max-length' : 'int',
> -'*x-bps-read' : 'int', '*x-bps-read-max' : 'int',
> -'*x-bps-read-max-length' : 'int', '*x-bps-write' : 'int',
> -'*x-bps-write-max' : 'int', '*x-bps-write-max-length' : 'int',
> -'*x-iops-size' : 'int' } }
> +'*x-iops-total': { 'type': 'int',
> +   'features': [ 'unstable' ] },
> +'*x-iops-total-max': { 'type': 'int',
> +   'features': [ 'unstable' ] },
> +'*x-iops-total-max-length': { 'type': 'int',
> +  'features': [ 'unstable' ] },
> +'*x-iops-read': { 'type': 'int',
> +  'features': [ 'unstable' ] },
> +'*x-iops-read-max': { 'type': 'int',
> + 

Re: [PATCH 1/9] qapi: New special feature flag "unstable"

[John Snow]

On Mon, Oct 25, 2021 at 1:26 AM Markus Armbruster  wrote:

> By convention, names starting with "x-" are experimental.  The parts
> of external interfaces so named may be withdrawn or changed
> incompatibly in future releases.
>
> Drawback: promoting something from experimental to stable involves a
> name change.  Client code needs to be updated.
>
> Moreover, the convention is not universally observed:
>
> * QOM type "input-barrier" has properties "x-origin", "y-origin".
>   Looks accidental, but it's ABI since 4.2.
>
> * QOM types "memory-backend-file", "memory-backend-memfd",
>   "memory-backend-ram", and "memory-backend-epc" have a property
>   "x-use-canonical-path-for-ramblock-id" that is documented to be
>   stable despite its name.
>
> We could document these exceptions, but documentation helps only
> humans.  We want to recognize "unstable" in code, like "deprecated".
>
> Replace the convention by a new special feature flag "unstable".  It
> will be recognized by the QAPI generator, like the existing feature
> flag "deprecated", and unlike regular feature flags.
>
> This commit updates documentation and prepares tests.  The next commit
> updates the QAPI schema.  The remaining patches update the QAPI
> generator and wire up -compat policy checking.
>
> Signed-off-by: Markus Armbruster 
> ---
>  docs/devel/qapi-code-gen.rst| 9 ++---
>  tests/qapi-schema/qapi-schema-test.json | 7 +--
>  tests/qapi-schema/qapi-schema-test.out  | 5 +
>  3 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
> index 4071c9074a..38f2d7aad3 100644
> --- a/docs/devel/qapi-code-gen.rst
> +++ b/docs/devel/qapi-code-gen.rst
> @@ -713,6 +713,10 @@ member as deprecated.  It is not supported elsewhere
> so far.
>  Interfaces so marked may be withdrawn in future releases in accordance
>  with QEMU's deprecation policy.
>
> +Feature "unstable" marks a command, event, enum value, or struct
> +member as unstable.  It is not supported elsewhere so far.  Interfaces
> +so marked may be withdrawn or changed incompatibly in future releases.
> +
>
>  Naming rules and reserved names
>  ---
> @@ -746,9 +750,8 @@ Member name ``u`` and names starting with ``has-`` or
> ``has_`` are reserved
>  for the generator, which uses them for unions and for tracking
>  optional members.
>
> -Any name (command, event, type, member, or enum value) beginning with
> -``x-`` is marked experimental, and may be withdrawn or changed
> -incompatibly in a future release.
> +Names beginning with ``x-`` used to signify "experimental".  This
> +convention has been replaced by special feature "unstable".
>
>  Pragmas ``command-name-exceptions`` and ``member-name-exceptions`` let
>  you violate naming rules.  Use for new code is strongly discouraged. See
> diff --git a/tests/qapi-schema/qapi-schema-test.json
> b/tests/qapi-schema/qapi-schema-test.json
> index b677ab861d..43b8697002 100644
> --- a/tests/qapi-schema/qapi-schema-test.json
> +++ b/tests/qapi-schema/qapi-schema-test.json
> @@ -273,7 +273,7 @@
>'data': { 'foo': { 'type': 'int', 'features': [ 'deprecated' ] } },
>'features': [ 'feature1' ] }
>  { 'struct': 'FeatureStruct2',
> -  'data': { 'foo': 'int' },
> +  'data': { 'foo': { 'type': 'int', 'features': [ 'unstable' ] } },
>'features': [ { 'name': 'feature1' } ] }
>  { 'struct': 'FeatureStruct3',
>'data': { 'foo': 'int' },
> @@ -331,7 +331,7 @@
>  { 'command': 'test-command-features1',
>'features': [ 'deprecated' ] }
>  { 'command': 'test-command-features3',
> -  'features': [ 'feature1', 'feature2' ] }
> +  'features': [ 'unstable', 'feature1', 'feature2' ] }
>
>  { 'command': 'test-command-cond-features1',
>'features': [ { 'name': 'feature1', 'if': 'TEST_IF_FEATURE_1'} ] }
> @@ -348,3 +348,6 @@
>
>  { 'event': 'TEST_EVENT_FEATURES1',
>'features': [ 'deprecated' ] }
> +
> +{ 'event': 'TEST_EVENT_FEATURES2',
> +  'features': [ 'unstable' ] }
> diff --git a/tests/qapi-schema/qapi-schema-test.out
> b/tests/qapi-schema/qapi-schema-test.out
> index 16846dbeb8..1f9585fa9b 100644
> --- a/tests/qapi-schema/qapi-schema-test.out
> +++ b/tests/qapi-schema/qapi-schema-test.out
> @@ -308,6 +308,7 @@ object FeatureStruct1
>  feature feature1
>  object FeatureStruct2
>  member foo: int optional=False
> +feature unstable
>  feature feature1
>  object FeatureStruct3
>  member foo: int optional=False
> @@ -373,6 +374,7 @@ command test-command-features1 None -> None
>  feature deprecated
>  command test-command-features3 None -> None
>  gen=True success_response=True boxed=False oob=False preconfig=False
> +feature unstable
>  feature feature1
>  feature feature2
>  command test-command-cond-features1 None -> None
> @@ -394,6 +396,9 @@ event TEST_EVENT_FEATURES0 FeatureStruct1
>  event TEST_EVENT_FEATURES1 None
>  boxed=False
>  feature deprecated
> +event 

[PATCH] docs: fix docs output path with meson 0.60.0

[Daniel P . Berrangé]

The meson 0.60.0 release introduced a bug with the '/' operator when
using an empty path component. '/foo' / ''  will now result in '/foo'
not '/foo/'

  https://github.com/mesonbuild/meson/issues/9450

This breaks libvirt because xsltproc requires the trailing '/' on the
output directory path. Fortunately the explicit 'join_paths' function
is not affected by the regression

Signed-off-by: Daniel P. Berrangé 
---

Pushed as a CI / build fix

 docs/meson.build | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/docs/meson.build b/docs/meson.build
index cbc138fa1f..fb6e0029d0 100644
--- a/docs/meson.build
+++ b/docs/meson.build
@@ -1,7 +1,9 @@
 docs_html_dir = docdir / 'html'
 
 # xsltproc requires that the -o path ends with '/'
-docs_builddir = meson.current_build_dir() / ''
+# Not using '/' operator due to bug in meson 0.60.0
+# https://github.com/mesonbuild/meson/issues/9450
+docs_builddir = join_paths(meson.current_build_dir(), '')
 
 docs_assets = [
   'android-chrome-192x192.png',
-- 
2.31.1

[libvirt PATCH] storage_file: Compute QCOW2 cluster size as ULL

[Jiri Denemark]

While the QCOW2 cluster size is represented in only 4 bits in the QCOW2
header and thus 1 << cluster_size cannot overflow int,
qcow2GetClusterSize is supposed to return unsigned long long so we can
just compute the result as ULL rather than computing it as int and
promoting to unsigned long long.

Signed-off-by: Jiri Denemark 
---
 src/storage_file/storage_file_probe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/storage_file/storage_file_probe.c 
b/src/storage_file/storage_file_probe.c
index f5bb8b7dbb..dc438a5e5d 100644
--- a/src/storage_file/storage_file_probe.c
+++ b/src/storage_file/storage_file_probe.c
@@ -487,7 +487,7 @@ qcow2GetClusterSize(const char *buf,
 clusterBits = virReadBufInt32BE(buf + QCOWX_HDR_CLUSTER_BITS_OFFSET);
 
 if (clusterBits > 0)
-return 1 << clusterBits;
+return 1ULL << clusterBits;
 
 return 0;
 }
-- 
2.33.1

[libvirt PATCH] node_device: Fix memory leak in udevProcessMediatedDevice

[Jiri Denemark]

One of the paths returned -1 directly without going through the cleanup
section.

Signed-off-by: Jiri Denemark 
---
 src/node_device/node_device_udev.c | 17 ++---
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/src/node_device/node_device_udev.c 
b/src/node_device/node_device_udev.c
index 7c3bb762b3..cd1722f934 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1020,10 +1020,9 @@ static int
 udevProcessMediatedDevice(struct udev_device *dev,
   virNodeDeviceDef *def)
 {
-int ret = -1;
 int iommugrp = -1;
-char *linkpath = NULL;
-char *canonicalpath = NULL;
+g_autofree char *linkpath = NULL;
+g_autofree char *canonicalpath = NULL;
 virNodeDevCapMdev *data = >caps->data.mdev;
 struct udev_device *parent_device = NULL;
 
@@ -1039,19 +1038,19 @@ udevProcessMediatedDevice(struct udev_device *dev,
 virReportSystemError(errno,
  _("failed to wait for file '%s' to appear"),
  linkpath);
-goto cleanup;
+return -1;
 }
 
 if (virFileResolveLink(linkpath, ) < 0) {
 virReportSystemError(errno, _("failed to resolve '%s'"), linkpath);
-goto cleanup;
+return -1;
 }
 
 data->type = g_path_get_basename(canonicalpath);
 
 data->uuid = g_strdup(udev_device_get_sysname(dev));
 if ((iommugrp = virMediatedDeviceGetIOMMUGroupNum(data->uuid)) < 0)
-goto cleanup;
+return -1;
 
 /* lookup the address of parent device */
 parent_device = udev_device_get_parent(dev);
@@ -1072,11 +1071,7 @@ udevProcessMediatedDevice(struct udev_device *dev,
 
 data->iommuGroupNumber = iommugrp;
 
-ret = 0;
- cleanup:
-VIR_FREE(linkpath);
-VIR_FREE(canonicalpath);
-return ret;
+return 0;
 }
 
 
-- 
2.33.1

[libvirt PATCH] util: Drop pointless NUL_TERMINATE macro

[Jiri Denemark]

It's only used once and open coding it is at least as clear as using the
macro.

Signed-off-by: Jiri Denemark 
---
 src/internal.h | 2 --
 src/util/virutil.c | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/internal.h b/src/internal.h
index e1250a59fe..d3809bf057 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -91,8 +91,6 @@
 #define STREQ_NULLABLE(a, b) (g_strcmp0(a, b) == 0)
 #define STRNEQ_NULLABLE(a, b) (g_strcmp0(a, b) != 0)
 
-#define NUL_TERMINATE(buf) do { (buf)[sizeof(buf)-1] = '\0'; } while (0)
-
 #ifdef WIN32
 # ifndef O_CLOEXEC
 #  define O_CLOEXEC _O_NOINHERIT
diff --git a/src/util/virutil.c b/src/util/virutil.c
index c9de043c40..e04f1343d8 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -485,7 +485,7 @@ virGetHostnameImpl(bool quiet)
  "%s", _("failed to determine host name"));
 return NULL;
 }
-NUL_TERMINATE(hostname);
+hostname[sizeof(hostname) - 1] = '\0';
 
 if (STRPREFIX(hostname, "localhost") || strchr(hostname, '.')) {
 /* in this case, gethostname returned localhost (meaning we can't
-- 
2.33.1

[PATCH] qemublocktest: Don't leak 'disk' in testQemuImageCreateLoadDiskXML

[Peter Krempa]

The function returns only the source portion but forgot to free the disk
wrapper.

Fixes: 9696427ad67196
Signed-off-by: Peter Krempa 
---
Pushed as a trivial CI fix.

 tests/qemublocktest.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 0176fbd3f4..9d3c2fe6b0 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -473,7 +473,7 @@ testQemuImageCreateLoadDiskXML(const char *name,
virDomainXMLOption *xmlopt)

 {
-virDomainDiskDef *disk = NULL;
+g_autoptr(virDomainDiskDef) disk = NULL;
 g_autofree char *xmlpath = NULL;
 g_autofree char *xmlstr = NULL;

@@ -490,7 +490,7 @@ testQemuImageCreateLoadDiskXML(const char *name,
 if (qemuDomainDeviceDiskDefPostParse(disk, 0) < 0)
 return NULL;

-return disk->src;
+return g_steal_pointer(>src);
 }


-- 
2.31.1

Re: [PATCH v5 0/5] Add support for librbd encryption

[Peter Krempa]

On Sun, Oct 24, 2021 at 04:51:25 -0500, Or Ozeri wrote:
> v5: rebased + nit fixes suggested by Petera

Series:

Reviewed-by: Peter Krempa 

and pushed.

[PATCH 1/2] qemu_monitor: Make domainMemoryDeviceSizeChange cb return void

[Michal Privoznik]

Nobody's interested in the return value of any of
struct _qemuMonitorCallbacks callbacks. They are all void, but
domainMemoryDeviceSizeChange. Change it to void.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_monitor.h | 10 +-
 src/qemu/qemu_process.c |  5 +
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 8214c2fd9f..b54c1cf87a 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -413,11 +413,11 @@ typedef void 
(*qemuMonitorDomainMemoryFailureCallback)(qemuMonitor *mon,

qemuMonitorEventMemoryFailure *mfp,
void *opaque);
 
-typedef int (*qemuMonitorDomainMemoryDeviceSizeChange)(qemuMonitor *mon,
-   virDomainObj *vm,
-   const char *alias,
-   unsigned long long size,
-   void *opaque);
+typedef void (*qemuMonitorDomainMemoryDeviceSizeChange)(qemuMonitor *mon,
+virDomainObj *vm,
+const char *alias,
+unsigned long long 
size,
+void *opaque);
 
 typedef struct _qemuMonitorCallbacks qemuMonitorCallbacks;
 struct _qemuMonitorCallbacks {
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 6027b30405..2a1fcad1ee 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1845,7 +1845,7 @@ qemuProcessHandleMemoryFailure(qemuMonitor *mon 
G_GNUC_UNUSED,
 }
 
 
-static int
+static void
 qemuProcessHandleMemoryDeviceSizeChange(qemuMonitor *mon G_GNUC_UNUSED,
 virDomainObj *vm,
 const char *devAlias,
@@ -1855,7 +1855,6 @@ qemuProcessHandleMemoryDeviceSizeChange(qemuMonitor *mon 
G_GNUC_UNUSED,
 virQEMUDriver *driver = opaque;
 struct qemuProcessEvent *processEvent = NULL;
 qemuMonitorMemoryDeviceSizeChange *info = NULL;
-int ret = -1;
 
 virObjectLock(vm);
 
@@ -1878,11 +1877,9 @@ qemuProcessHandleMemoryDeviceSizeChange(qemuMonitor *mon 
G_GNUC_UNUSED,
 }
 
 processEvent = NULL;
-ret = 0;
  cleanup:
 qemuProcessEventFree(processEvent);
 virObjectUnlock(vm);
-return ret;
 }
 
 
-- 
2.32.0

[PATCH 0/2] domainMemoryDeviceSizeChange: Two simple improvements

[Michal Privoznik]

*** BLURB HERE ***

Michal Prívozník (2):
  qemu_monitor: Make domainMemoryDeviceSizeChange cb return void
  qemuProcessHandleMemoryDeviceSizeChange: Use qemuProcessEventSubmit()

 src/qemu/qemu_monitor.h | 10 +-
 src/qemu/qemu_process.c | 14 ++
 2 files changed, 7 insertions(+), 17 deletions(-)

-- 
2.32.0

[PATCH 2/2] qemuProcessHandleMemoryDeviceSizeChange: Use qemuProcessEventSubmit()

[Michal Privoznik]

This is a typical example of what can go wrong when sending out
an old patch. Back in January, when I was writing
qemuProcessHandleMemoryDeviceSizeChange() events were sent to the
worker pool thread using virThreadPoolSendJob(). Then, in July a
helper was introduced (qemuProcessEventSubmit()) but since my
code was not committed and I did not pay attention my code wasn't
updated. Later, when I merged my code it uses the old approach.

BTW: this also fixes a possible double free which I completely
missed when writing the code ~10 months ago.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_process.c | 9 +
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 2a1fcad1ee..d5f8a47ac2 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1870,15 +1870,8 @@ qemuProcessHandleMemoryDeviceSizeChange(qemuMonitor *mon 
G_GNUC_UNUSED,
 processEvent->vm = virObjectRef(vm);
 processEvent->data = g_steal_pointer();
 
-if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) {
-qemuProcessEventFree(processEvent);
-virObjectUnref(vm);
-goto cleanup;
-}
+qemuProcessEventSubmit(driver, );
 
-processEvent = NULL;
- cleanup:
-qemuProcessEventFree(processEvent);
 virObjectUnlock(vm);
 }
 
-- 
2.32.0

Re: [PATCH 0/5] trace: inroduce qmp: trace namespace

[Stefan Hajnoczi]

On Thu, Oct 14, 2021 at 06:22:32PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 12.10.2021 14:49, Markus Armbruster wrote:
> > Vladimir Sementsov-Ogievskiy  writes:
> > 
> > > Hi all!
> > > 
> > > We have handle_qmp_command and qmp_command_repond trace points to trace
> > > qmp commands. They are very useful to debug problems involving
> > > management tools like libvirt.
> > > 
> > > But tracing all qmp commands is too much.
> > > 
> > > Here I suggest a kind of tracing namespace. Formally this series adds a
> > > trace points called qmp: for every command, which may be
> > > enabled in separate like
> > > 
> > >--trace qmp:drive-backup
> > > 
> > > or by pattern like
> > > 
> > >--trace qmp:block-job-*
> > > 
> > > or similarly with help of qmp command trace-event-set-state.
> > > 
> > > This also allows to enable tracing of some qmp commands permanently
> > >   (by downstream patch or in libvirt xml). For example, I'm going to
> > > enable tracing of block job comamnds and blockdev-* commands in
> > > Virtuozzo. Qemu logs are often too empty (for example, in comparison
> > > with Libvirt), logging block jobs is not too much but will be very
> > > helpful.
> > 
> > What exactly is traced?  Peeking at PATCH 5... looks like it's input
> > that makes it to qmp_dispatch() and command responses, but not events.
> > 
> > Fine print on "input that makes it to qmp_dispatch()":
> > 
> > * You trace right before we execute the command, not when we receive,
> >parse and enqueue input.
> > 
> > * Corollary: input with certain errors is not traced.
> > 
> > * You don't trace the input text, you trace the unparsed parse tree.
> > 
> > All fine, I presume.
> > 
> > Existing tracepoints in monitor/qmp.c, and what information they send
> > (inessential bits omitted for clarity):
> > 
> > * handle_qmp_command
> > 
> >Handling a QMP command: unparsed parse tree
> > 
> >Fine print, safe to ignore:
> > 
> >- Out-of-band commands will be executed right away, in-band commands
> >  will be queued.  Tracepoints monitor_qmp_in_band_enqueue and
> >  monitor_qmp_in_band_dequeue provide insight into that.
> > 
> >- This also receives and queues parse errors, without tracing them.
> >  Tracepoint monitor_qmp_err_in_band traces them as they are dequeued.
> > 
> > * monitor_qmp_cmd_in_band
> > 
> >About to execute in-band command: command ID, if any
> > 
> > * monitor_qmp_cmd_out_of_band
> > 
> >About to execute out-of-band command: command ID, if any
> > 
> > * monitor_qmp_respond
> > 
> >About to send command response or event: QObject
> > 
> > For input, --trace qmp:* is like --trace handle_qmp_command, except it
> > traces late rather than early.
> > 
> > For output, --trace qmp:* is like --trace monitor_qmp_respond less
> > events.
> > 
> > The main improvement over existing tracepoints seems to be the ability
> > to filter on command names.
> > 
> > To get that, you overload the @name argument of QMP command
> > trace-event-set-state.  In addition to the documented meaning "Event
> > name pattern", it also has an alternate, undocumented meaning "QMP
> > command name pattern".  The "undocumented" part is easy enough to fix.
> > However, QMP heavily frowns on argument values that need to be parsed.
> 
> Still, pattern is parsed anyway, as pattern. But yes, this patch adds
> rather specific and tricky logic, which a lot more than just a pattern
> to search through the list.
> 
> Another possible way is to update QAPI code generator to insert a personal
> trace point for each qmp command.. That seems more complicated to implement,
> but I can try.

That's what came to mind when I saw this series too. The QAPI generator
can create a trace event for each QMP command. That way each command has
a dedicated trace event that can be enabled/disabled in the usual way
(e.g. built-in "trace" monitor command, SystemTap scripts, etc) without
introducing special syntax.

Stefan


signature.asc
Description: PGP signature

Re: [PATCH 1/9] qapi: New special feature flag "unstable"

[Philippe Mathieu-Daudé]

On 10/25/21 14:05, Kashyap Chamarthy wrote:
> On Mon, Oct 25, 2021 at 07:25:24AM +0200, Markus Armbruster wrote:
>> By convention, names starting with "x-" are experimental.  The parts
>> of external interfaces so named may be withdrawn or changed
>> incompatibly in future releases.
>>
>> Drawback: promoting something from experimental to stable involves a
>> name change.  Client code needs to be updated.
>>
>> Moreover, the convention is not universally observed:
>>
>> * QOM type "input-barrier" has properties "x-origin", "y-origin".
>>   Looks accidental, but it's ABI since 4.2.
>>
>> * QOM types "memory-backend-file", "memory-backend-memfd",
>>   "memory-backend-ram", and "memory-backend-epc" have a property
>>   "x-use-canonical-path-for-ramblock-id" that is documented to be
>>   stable despite its name.
> 
> Looks like there's another stable property with an "x-" prefix:
> "x-remote-object", part of QOM type @RemoteObjectProperties.

IIRC "x-remote-object" and RemoteObjectProperties are not stable.

Re: [PATCH 8/9] qapi: Factor out compat_policy_input_ok()

[Philippe Mathieu-Daudé]

On 10/25/21 07:25, Markus Armbruster wrote:
> The code to check policy for handling deprecated input is triplicated.
> Factor it out into compat_policy_input_ok() before I mess with it in
> the next commit.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  include/qapi/compat-policy.h |  7 +
>  qapi/qapi-visit-core.c   | 18 +
>  qapi/qmp-dispatch.c  | 51 +++-
>  qapi/qobject-input-visitor.c | 19 +++---
>  4 files changed, 55 insertions(+), 40 deletions(-)

> diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
> index 8cca18c891..e29ade134c 100644
> --- a/qapi/qmp-dispatch.c
> +++ b/qapi/qmp-dispatch.c
> @@ -28,6 +28,40 @@
>  
>  CompatPolicy compat_policy;
>  
> +static bool compat_policy_input_ok1(const char *adjective,
> +CompatPolicyInput policy,
> +ErrorClass error_class,
> +const char *kind, const char *name,
> +Error **errp)
> +{
> +switch (policy) {
> +case COMPAT_POLICY_INPUT_ACCEPT:
> +return true;
> +case COMPAT_POLICY_INPUT_REJECT:
> +error_set(errp, error_class, "%s %s %s disabled by policy",
> +  adjective, kind, name);
> +return false;
> +case COMPAT_POLICY_INPUT_CRASH:
> +default:
> +abort();

g_assert_not_reached() provides a nicer user experience.

> +}
> +}
> +
> +bool compat_policy_input_ok(unsigned special_features,
> +const CompatPolicy *policy,
> +ErrorClass error_class,
> +const char *kind, const char *name,
> +Error **errp)
> +{
> +if ((special_features & 1u << QAPI_DEPRECATED)

Matter of taste, I find code using extract() easier to review:

  extract64(special_features, QAPI_DEPRECATED, 1)

> +&& !compat_policy_input_ok1("Deprecated",
> +policy->deprecated_input,
> +error_class, kind, name, errp)) {
> +return false;
> +}
> +return true;
> +}

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH] ci: regenerate container from manifest

[Daniel P . Berrangé]

On Mon, Oct 25, 2021 at 02:06:16PM +0200, Tim Wiederhake wrote:
> On Mon, 2021-10-25 at 12:46 +0100, Daniel P. Berrangé wrote:
> > This removes the libnetcf-dev package from Debian Sid, as it is no
> > longer available in that distro stream.
> > 
> > Signed-off-by: Daniel P. Berrangé 
> > ---
> > 
> >  * Pushed as a CI build fix
> > 
> >  ci/containers/debian-sid.Dockerfile | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/ci/containers/debian-sid.Dockerfile
> > b/ci/containers/debian-sid.Dockerfile
> > index d8667c5f1b..16dd6cc587 100644
> > --- a/ci/containers/debian-sid.Dockerfile
> > +++ b/ci/containers/debian-sid.Dockerfile
> > @@ -46,7 +46,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
> >  libglusterfs-dev \
> >  libgnutls28-dev \
> >  libiscsi-dev \
> > -    libnetcf-dev \
> >  libnl-3-dev \
> >  libnl-route-3-dev \
> >  libnuma-dev \
> 
> libnetcf-dev has also been removed from Debian Testing (Bookworm / 11-to-be).

FWIW, we don't track that as a separate distro in libvirt-ci just now.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [PATCH] ci: regenerate container from manifest

[Tim Wiederhake]

On Mon, 2021-10-25 at 12:46 +0100, Daniel P. Berrangé wrote:
> This removes the libnetcf-dev package from Debian Sid, as it is no
> longer available in that distro stream.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
> 
>  * Pushed as a CI build fix
> 
>  ci/containers/debian-sid.Dockerfile | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/ci/containers/debian-sid.Dockerfile
> b/ci/containers/debian-sid.Dockerfile
> index d8667c5f1b..16dd6cc587 100644
> --- a/ci/containers/debian-sid.Dockerfile
> +++ b/ci/containers/debian-sid.Dockerfile
> @@ -46,7 +46,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
>  libglusterfs-dev \
>  libgnutls28-dev \
>  libiscsi-dev \
> -    libnetcf-dev \
>  libnl-3-dev \
>  libnl-route-3-dev \
>  libnuma-dev \

libnetcf-dev has also been removed from Debian Testing (Bookworm / 11-to-be).

Reviewed-by: Tim Wiederhake 

Re: [PATCH 1/9] qapi: New special feature flag "unstable"

[Kashyap Chamarthy]

On Mon, Oct 25, 2021 at 07:25:24AM +0200, Markus Armbruster wrote:
> By convention, names starting with "x-" are experimental.  The parts
> of external interfaces so named may be withdrawn or changed
> incompatibly in future releases.
> 
> Drawback: promoting something from experimental to stable involves a
> name change.  Client code needs to be updated.
> 
> Moreover, the convention is not universally observed:
> 
> * QOM type "input-barrier" has properties "x-origin", "y-origin".
>   Looks accidental, but it's ABI since 4.2.
> 
> * QOM types "memory-backend-file", "memory-backend-memfd",
>   "memory-backend-ram", and "memory-backend-epc" have a property
>   "x-use-canonical-path-for-ramblock-id" that is documented to be
>   stable despite its name.

Looks like there's another stable property with an "x-" prefix:
"x-remote-object", part of QOM type @RemoteObjectProperties.

Given the above "x-" properties are now stable, I take it that they
cannot be renamed now, as they might break any tools using them?  My
guess is the tedious way is not worth it: deprecate them, and add the
non-x variants as "synonyms".  
 
> We could document these exceptions, but documentation helps only
> humans.  We want to recognize "unstable" in code, like "deprecated".
>
> Replace the convention by a new special feature flag "unstable".  It
> will be recognized by the QAPI generator, like the existing feature
> flag "deprecated", and unlike regular feature flags.

FWIW, sounds good to me.

> This commit updates documentation and prepares tests.  The next commit
> updates the QAPI schema.  The remaining patches update the QAPI
> generator and wire up -compat policy checking.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  docs/devel/qapi-code-gen.rst| 9 ++---
>  tests/qapi-schema/qapi-schema-test.json | 7 +--
>  tests/qapi-schema/qapi-schema-test.out  | 5 +
>  3 files changed, 16 insertions(+), 5 deletions(-)

[...]

-- 
/kashyap

Re: [PATCH 6/9] qapi: Generalize command policy checking

[Philippe Mathieu-Daudé]

On 10/25/21 07:25, Markus Armbruster wrote:
> The code to check command policy can see special feature flag
> 'deprecated' as command flag QCO_DEPRECATED.  I want to make feature
> flag 'unstable' visible there as well, so I can add policy for it.
> 
> To let me make it visible, add member @special_features (a bitset of
> QapiSpecialFeature) to QmpCommand, and adjust the generator to pass it
> through qmp_register_command().  Then replace "QCO_DEPRECATED in
> @flags" by QAPI_DEPRECATED in @special_features", and drop
> QCO_DEPRECATED.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  include/qapi/qmp/dispatch.h  | 5 +++--
>  monitor/misc.c   | 6 --
>  qapi/qmp-dispatch.c  | 2 +-
>  qapi/qmp-registry.c  | 4 +++-
>  storage-daemon/qemu-storage-daemon.c | 3 ++-
>  scripts/qapi/commands.py | 9 -
>  6 files changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
> index 0ce88200b9..1e4240fd0d 100644
> --- a/include/qapi/qmp/dispatch.h
> +++ b/include/qapi/qmp/dispatch.h
> @@ -25,7 +25,6 @@ typedef enum QmpCommandOptions
>  QCO_ALLOW_OOB =  (1U << 1),
>  QCO_ALLOW_PRECONFIG   =  (1U << 2),
>  QCO_COROUTINE =  (1U << 3),
> -QCO_DEPRECATED=  (1U << 4),
>  } QmpCommandOptions;
>  
>  typedef struct QmpCommand
> @@ -34,6 +33,7 @@ typedef struct QmpCommand
>  /* Runs in coroutine context if QCO_COROUTINE is set */
>  QmpCommandFunc *fn;
>  QmpCommandOptions options;
> +unsigned special_features;
>  QTAILQ_ENTRY(QmpCommand) node;
>  bool enabled;
>  const char *disable_reason;
> @@ -42,7 +42,8 @@ typedef struct QmpCommand
>  typedef QTAILQ_HEAD(QmpCommandList, QmpCommand) QmpCommandList;
>  
>  void qmp_register_command(QmpCommandList *cmds, const char *name,
> -  QmpCommandFunc *fn, QmpCommandOptions options);
> +  QmpCommandFunc *fn, QmpCommandOptions options,
> +  unsigned special_features);

What about:

  typedef unsigned QapiFeatureMask;

?

Otherwise:
Reviewed-by: Philippe Mathieu-Daudé 

[PATCH] ci: regenerate container from manifest

[Daniel P . Berrangé]

This removes the libnetcf-dev package from Debian Sid, as it is no
longer available in that distro stream.

Signed-off-by: Daniel P. Berrangé 
---

 * Pushed as a CI build fix

 ci/containers/debian-sid.Dockerfile | 1 -
 1 file changed, 1 deletion(-)

diff --git a/ci/containers/debian-sid.Dockerfile 
b/ci/containers/debian-sid.Dockerfile
index d8667c5f1b..16dd6cc587 100644
--- a/ci/containers/debian-sid.Dockerfile
+++ b/ci/containers/debian-sid.Dockerfile
@@ -46,7 +46,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
 libglusterfs-dev \
 libgnutls28-dev \
 libiscsi-dev \
-libnetcf-dev \
 libnl-3-dev \
 libnl-route-3-dev \
 libnuma-dev \
-- 
2.31.1

Re: [PATCH 00/13] qemu_monitor_json: Use g_auto* more

[Tim Wiederhake]

On Mon, 2021-10-25 at 12:57 +0200, Michal Privoznik wrote:
> *** BLURB HERE ***
> 
> Michal Prívozník (13):
>   qemu_monitor_json: Don't check for qemuMonitorNextCommandID()
> retval
>   qemu_monitor_json: Don't transfer ownership to @msg
>   qemuMonitorJSONHumanCommand: Require @reply_str
>   qemuMonitorJSONGetMigrationStats: Don't clear @stats on failure
>   qemuMonitorJSONQueryRxFilterParse: Set *filter only on success
>   qemu_monitor: Declare and use g_autoptr for
> qemuMonitorEventPanicInfo
>   qemu_monitor_json: Use g_autoptr() for virCPUData
>   qemu_monitor_json: Use g_autoptr() for qemuMonitorCPUModelInfo
>   qemuMonitorJSONExtractPRManagerInfo: Declare @entry inside the loop
>   qemu_monitor_json: Use g_autoptr() for virJSONValue
>   qemu_monitor_json: Use g_autofree
>   qemu_monitor_json: Drop pointless cleanup labels
>   qemu_monitor_json: Drop pointless error labels
> 
>  src/qemu/qemu_monitor.c  |    2 +-
>  src/qemu/qemu_monitor.h  |    3 +-
>  src/qemu/qemu_monitor_json.c | 1803 
> --
>  3 files changed, 641 insertions(+), 1167 deletions(-)
> 

> 1167 deletions
Thanks!

Reviewed-by: Tim Wiederhake 

Tim

[PATCH 08/13] qemu_monitor_json: Use g_autoptr() for qemuMonitorCPUModelInfo

[Michal Privoznik]

There's one place (specifically qemuMonitorJSONParseCPUModel())
where we can avoid explicit free call for qemuMonitorCPUModelInfo
struct.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_monitor_json.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 3d89afa6c6..f59688dfd5 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5693,7 +5693,7 @@ qemuMonitorJSONParseCPUModel(const char *cpu_name,
  virJSONValue *cpu_props,
  qemuMonitorCPUModelInfo **model_info)
 {
-qemuMonitorCPUModelInfo *machine_model = NULL;
+g_autoptr(qemuMonitorCPUModelInfo) machine_model = NULL;
 int ret = -1;
 
 machine_model = g_new0(qemuMonitorCPUModelInfo, 1);
@@ -5714,7 +5714,6 @@ qemuMonitorJSONParseCPUModel(const char *cpu_name,
 ret = 0;
 
  cleanup:
-qemuMonitorCPUModelInfoFree(machine_model);
 return ret;
 }
 
-- 
2.32.0

[PATCH 12/13] qemu_monitor_json: Drop pointless cleanup labels

[Michal Privoznik]

After previous cleanups, some 'cleanup' labels were rendered
needless - they contain nothing more than a return statement.
Well, those labels can be dropped and 'goto cleanup' can be
replaced with return statement directly.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_monitor_json.c | 1035 --
 1 file changed, 368 insertions(+), 667 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index cd4a37a685..bb76ac7d37 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -216,25 +216,24 @@ qemuMonitorJSONIOProcessLine(qemuMonitor *mon,
  qemuMonitorMessage *msg)
 {
 g_autoptr(virJSONValue) obj = NULL;
-int ret = -1;
 
 VIR_DEBUG("Line [%s]", line);
 
 if (!(obj = virJSONValueFromString(line)))
-goto cleanup;
+return -1;
 
 if (virJSONValueGetType(obj) != VIR_JSON_TYPE_OBJECT) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Parsed JSON reply '%s' isn't an object"), line);
-goto cleanup;
+return -1;
 }
 
 if (virJSONValueObjectHasKey(obj, "QMP") == 1) {
-ret = 0;
+return 0;
 } else if (virJSONValueObjectHasKey(obj, "event") == 1) {
 PROBE(QEMU_MONITOR_RECV_EVENT,
   "mon=%p event=%s", mon, line);
-ret = qemuMonitorJSONIOProcessEvent(mon, obj);
+return qemuMonitorJSONIOProcessEvent(mon, obj);
 } else if (virJSONValueObjectHasKey(obj, "error") == 1 ||
virJSONValueObjectHasKey(obj, "return") == 1) {
 PROBE(QEMU_MONITOR_RECV_REPLY,
@@ -242,7 +241,7 @@ qemuMonitorJSONIOProcessLine(qemuMonitor *mon,
 if (msg) {
 msg->rxObject = g_steal_pointer();
 msg->finished = 1;
-ret = 0;
+return 0;
 } else {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unexpected JSON reply '%s'"), line);
@@ -252,8 +251,7 @@ qemuMonitorJSONIOProcessLine(qemuMonitor *mon,
_("Unknown JSON reply '%s'"), line);
 }
 
- cleanup:
-return ret;
+return -1;
 }
 
 int qemuMonitorJSONIOProcess(qemuMonitor *mon,
@@ -307,12 +305,12 @@ qemuMonitorJSONCommandWithFd(qemuMonitor *mon,
 if (virJSONValueObjectAppendString(cmd, "id", id) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Unable to append command 'id' string"));
-goto cleanup;
+return -1;
 }
 }
 
 if (virJSONValueToBuffer(cmd, , false) < 0)
-goto cleanup;
+return -1;
 virBufferAddLit(, "\r\n");
 
 msg.txLength = virBufferUse();
@@ -331,7 +329,6 @@ qemuMonitorJSONCommandWithFd(qemuMonitor *mon,
 }
 }
 
- cleanup:
 return ret;
 }
 
@@ -1455,33 +1452,29 @@ qemuMonitorJSONHumanCommand(qemuMonitor *mon,
 g_autoptr(virJSONValue) reply = NULL;
 virJSONValue *obj;
 const char *data;
-int ret = -1;
 
 cmd = qemuMonitorJSONMakeCommand("human-monitor-command",
  "s:command-line", cmd_str,
  NULL);
 
 if (!cmd || qemuMonitorJSONCommand(mon, cmd, ) < 0)
-goto cleanup;
+return -1;
 
 if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
_("Human monitor command is not available to run %s"),
cmd_str);
-goto cleanup;
+return -1;
 }
 
 if (qemuMonitorJSONCheckError(cmd, reply))
-goto cleanup;
+return -1;
 
 obj = virJSONValueObjectGet(reply, "return");
 data = virJSONValueGetString(obj);
 *reply_str = g_strdup(NULLSTR_EMPTY(data));
 
-ret = 0;
-
- cleanup:
-return ret;
+return 0;
 }
 
 
@@ -1542,7 +1535,6 @@ qemuMonitorJSONStartCPUs(qemuMonitor *mon)
 int
 qemuMonitorJSONStopCPUs(qemuMonitor *mon)
 {
-int ret = -1;
 g_autoptr(virJSONValue) cmd = qemuMonitorJSONMakeCommand("stop", NULL);
 g_autoptr(virJSONValue) reply = NULL;
 
@@ -1550,14 +1542,12 @@ qemuMonitorJSONStopCPUs(qemuMonitor *mon)
 return -1;
 
 if (qemuMonitorJSONCommand(mon, cmd, ) < 0)
-goto cleanup;
+return -1;
 
 if (qemuMonitorJSONCheckError(cmd, reply) < 0)
-goto cleanup;
+return -1;
 
-ret = 0;
- cleanup:
-return ret;
+return 0;
 }
 
 
@@ -1604,7 +1594,6 @@ qemuMonitorJSONGetStatus(qemuMonitor *mon,
 
 int qemuMonitorJSONSystemPowerdown(qemuMonitor *mon)
 {
-int ret = -1;
 g_autoptr(virJSONValue) cmd = 
qemuMonitorJSONMakeCommand("system_powerdown", NULL);
 g_autoptr(virJSONValue) reply = NULL;
 
@@ -1612,21 +1601,18 @@ int qemuMonitorJSONSystemPowerdown(qemuMonitor *mon)
 return -1;
 
 if (qemuMonitorJSONCommand(mon, cmd, ) < 0)
-goto cleanup;
+return -1;
 
 if (qemuMonitorJSONCheckError(cmd, 

[PATCH 09/13] qemuMonitorJSONExtractPRManagerInfo: Declare @entry inside the loop

[Michal Privoznik]

The reason why @entry variable in qemuMonitorJSONExtractPRManagerInfo()
was declared at the top most level was that the variable is used under
the cleanup label.  However, if declared using g_autofree then the
variable can be declared inside the loop it is used in.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_monitor_json.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index f59688dfd5..f7bc680035 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -8622,7 +8622,6 @@ static int
 qemuMonitorJSONExtractPRManagerInfo(virJSONValue *reply,
 GHashTable *info)
 {
-qemuMonitorPRManagerInfo *entry = NULL;
 virJSONValue *data;
 int ret = -1;
 size_t i;
@@ -8630,6 +8629,7 @@ qemuMonitorJSONExtractPRManagerInfo(virJSONValue *reply,
 data = virJSONValueObjectGetArray(reply, "return");
 
 for (i = 0; i < virJSONValueArraySize(data); i++) {
+g_autofree qemuMonitorPRManagerInfo *entry = NULL;
 virJSONValue *prManager = virJSONValueArrayGet(data, i);
 const char *alias;
 
@@ -8652,7 +8652,6 @@ qemuMonitorJSONExtractPRManagerInfo(virJSONValue *reply,
 
 ret = 0;
  cleanup:
-VIR_FREE(entry);
 return ret;
 
  malformed:
-- 
2.32.0

[PATCH 11/13] qemu_monitor_json: Use g_autofree

[Michal Privoznik]

Let's replace VIR_FREE() calls with g_autofree. Not all calls can
be replaced though - the legitimate ones are kept (e.g. those
which free array, or which free a struct for which we don't have
g_autoptr() yet, and so on).

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_monitor_json.c | 37 +++-
 1 file changed, 11 insertions(+), 26 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 1eb0b3b54c..cd4a37a685 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -175,7 +175,7 @@ qemuMonitorJSONIOProcessEvent(qemuMonitor *mon,
 const char *type;
 qemuEventHandler *handler;
 virJSONValue *data;
-char *details = NULL;
+g_autofree char *details = NULL;
 virJSONValue *timestamp;
 long long seconds = -1;
 unsigned int micros = 0;
@@ -199,7 +199,6 @@ qemuMonitorJSONIOProcessEvent(qemuMonitor *mon,
  ));
 }
 qemuMonitorEmitEvent(mon, type, seconds, micros, details);
-VIR_FREE(details);
 
 handler = bsearch(type, eventHandlers, G_N_ELEMENTS(eventHandlers),
   sizeof(eventHandlers[0]), qemuMonitorEventCompare);
@@ -270,16 +269,12 @@ int qemuMonitorJSONIOProcess(qemuMonitor *mon,
 
 if (nl) {
 int got = nl - (data + used);
-char *line;
-line = g_strndup(data + used, got);
+g_autofree char *line = g_strndup(data + used, got);
+
 used += got + strlen(LINE_ENDING);
 line[got] = '\0'; /* kill \n */
-if (qemuMonitorJSONIOProcessLine(mon, line, msg) < 0) {
-VIR_FREE(line);
+if (qemuMonitorJSONIOProcessLine(mon, line, msg) < 0)
 return -1;
-}
-
-VIR_FREE(line);
 } else {
 break;
 }
@@ -6458,7 +6453,7 @@ qemuMonitorJSONGetGICCapabilities(qemuMonitor *mon,
 g_autoptr(virJSONValue) cmd = NULL;
 g_autoptr(virJSONValue) reply = NULL;
 virJSONValue *caps;
-virGICCapability *list = NULL;
+g_autofree virGICCapability *list = NULL;
 size_t i;
 size_t n;
 
@@ -6534,8 +6529,6 @@ qemuMonitorJSONGetGICCapabilities(qemuMonitor *mon,
 *capabilities = g_steal_pointer();
 
  cleanup:
-VIR_FREE(list);
-
 return ret;
 }
 
@@ -6674,7 +6667,7 @@ qemuMonitorJSONNBDServerStart(qemuMonitor *mon,
 g_autoptr(virJSONValue) cmd = NULL;
 g_autoptr(virJSONValue) reply = NULL;
 g_autoptr(virJSONValue) addr = NULL;
-char *port_str = NULL;
+g_autofree char *port_str = NULL;
 
 switch ((virStorageNetHostTransport)server->transport) {
 case VIR_STORAGE_NET_HOST_TRANS_TCP:
@@ -6708,7 +6701,6 @@ qemuMonitorJSONNBDServerStart(qemuMonitor *mon,
 ret = 0;
 
  cleanup:
-VIR_FREE(port_str);
 return ret;
 }
 
@@ -6848,7 +6840,7 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
 const char *backend_type = NULL;
 const char *host;
 const char *port;
-char *tlsalias = NULL;
+g_autofree char *tlsalias = NULL;
 bool telnet;
 
 switch ((virDomainChrType)chr->type) {
@@ -6997,7 +6989,6 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
 goto cleanup;
 
  cleanup:
-VIR_FREE(tlsalias);
 return ret;
 }
 
@@ -7568,7 +7559,7 @@ qemuMonitorJSONSetIOThread(qemuMonitor *mon,
qemuMonitorIOThreadInfo *iothreadInfo)
 {
 int ret = -1;
-char *path = NULL;
+g_autofree char *path = NULL;
 qemuMonitorJSONObjectProperty prop;
 
 path = g_strdup_printf("/objects/iothread%u", iothreadInfo->iothread_id);
@@ -7591,7 +7582,6 @@ qemuMonitorJSONSetIOThread(qemuMonitor *mon,
 ret = 0;
 
  cleanup:
-VIR_FREE(path);
 return ret;
 }
 
@@ -7724,7 +7714,7 @@ qemuMonitorJSONFindObjectPathByAlias(qemuMonitor *mon,
  char **path)
 {
 qemuMonitorJSONListPath **paths = NULL;
-char *child = NULL;
+g_autofree char *child = NULL;
 int npaths;
 int ret = -1;
 size_t i;
@@ -7750,7 +7740,6 @@ qemuMonitorJSONFindObjectPathByAlias(qemuMonitor *mon,
 for (i = 0; i < npaths; i++)
 qemuMonitorJSONListPathFree(paths[i]);
 VIR_FREE(paths);
-VIR_FREE(child);
 return ret;
 }
 
@@ -7775,7 +7764,6 @@ qemuMonitorJSONFindObjectPathByName(qemuMonitor *mon,
 {
 ssize_t i, npaths = 0;
 int ret = -2;
-char *nextpath = NULL;
 qemuMonitorJSONListPath **paths = NULL;
 
 VIR_DEBUG("Searching for '%s' Object Path starting at '%s'", name, 
curpath);
@@ -7797,10 +7785,9 @@ qemuMonitorJSONFindObjectPathByName(qemuMonitor *mon,
  * traversed looking for more entries
  */
 if (paths[i]->type && STRPREFIX(paths[i]->type, "child<")) {
-nextpath = g_strdup_printf("%s/%s", curpath, paths[i]->name);
+g_autofree char *nextpath = g_strdup_printf("%s/%s", curpath, 
paths[i]->name);
 
 ret = 

[PATCH 10/13] qemu_monitor_json: Use g_autoptr() for virJSONValue

[Michal Privoznik]

A lot of explicit free calls can be saved when virJSONValue
variables are declared with g_autoptr(). There's one caveat:
there was a slight deviation from our usual pattern such that
@cmd variable was not initialized to NULL but as the very first
step it was assigned a value using qemuMonitorJSONMakeCommand().
While this works in theory it upset my GCC-11.2 (but only when
building with -O2). So I had to initialize the variable in such
case too.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_monitor_json.c | 623 +--
 1 file changed, 227 insertions(+), 396 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index f7bc680035..1eb0b3b54c 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -216,7 +216,7 @@ qemuMonitorJSONIOProcessLine(qemuMonitor *mon,
  const char *line,
  qemuMonitorMessage *msg)
 {
-virJSONValue *obj = NULL;
+g_autoptr(virJSONValue) obj = NULL;
 int ret = -1;
 
 VIR_DEBUG("Line [%s]", line);
@@ -254,7 +254,6 @@ qemuMonitorJSONIOProcessLine(qemuMonitor *mon,
 }
 
  cleanup:
-virJSONValueFree(obj);
 return ret;
 }
 
@@ -1457,8 +1456,8 @@ qemuMonitorJSONHumanCommand(qemuMonitor *mon,
 const char *cmd_str,
 char **reply_str)
 {
-virJSONValue *cmd = NULL;
-virJSONValue *reply = NULL;
+g_autoptr(virJSONValue) cmd = NULL;
+g_autoptr(virJSONValue) reply = NULL;
 virJSONValue *obj;
 const char *data;
 int ret = -1;
@@ -1487,8 +1486,6 @@ qemuMonitorJSONHumanCommand(qemuMonitor *mon,
 ret = 0;
 
  cleanup:
-virJSONValueFree(cmd);
-virJSONValueFree(reply);
 return ret;
 }
 
@@ -1517,14 +1514,15 @@ int
 qemuMonitorJSONStartCPUs(qemuMonitor *mon)
 {
 int ret;
-virJSONValue *cmd = qemuMonitorJSONMakeCommand("cont", NULL);
-virJSONValue *reply = NULL;
+g_autoptr(virJSONValue) cmd = qemuMonitorJSONMakeCommand("cont", NULL);
 size_t i = 0;
 int timeout = 3;
 if (!cmd)
 return -1;
 
 do {
+g_autoptr(virJSONValue) reply = NULL;
+
 ret = qemuMonitorJSONCommand(mon, cmd, );
 
 if (ret != 0)
@@ -1539,13 +1537,9 @@ qemuMonitorJSONStartCPUs(qemuMonitor *mon)
 if (!qemuMonitorJSONHasError(reply, "MigrationExpected"))
 break;
 
-virJSONValueFree(reply);
-reply = NULL;
 g_usleep(25);
 } while (++i <= timeout);
 
-virJSONValueFree(cmd);
-virJSONValueFree(reply);
 return ret;
 }
 
@@ -1554,8 +1548,9 @@ int
 qemuMonitorJSONStopCPUs(qemuMonitor *mon)
 {
 int ret = -1;
-virJSONValue *cmd = qemuMonitorJSONMakeCommand("stop", NULL);
-virJSONValue *reply = NULL;
+g_autoptr(virJSONValue) cmd = qemuMonitorJSONMakeCommand("stop", NULL);
+g_autoptr(virJSONValue) reply = NULL;
+
 if (!cmd)
 return -1;
 
@@ -1567,8 +1562,6 @@ qemuMonitorJSONStopCPUs(qemuMonitor *mon)
 
 ret = 0;
  cleanup:
-virJSONValueFree(cmd);
-virJSONValueFree(reply);
 return ret;
 }
 
@@ -1617,8 +1610,9 @@ qemuMonitorJSONGetStatus(qemuMonitor *mon,
 int qemuMonitorJSONSystemPowerdown(qemuMonitor *mon)
 {
 int ret = -1;
-virJSONValue *cmd = qemuMonitorJSONMakeCommand("system_powerdown", NULL);
-virJSONValue *reply = NULL;
+g_autoptr(virJSONValue) cmd = 
qemuMonitorJSONMakeCommand("system_powerdown", NULL);
+g_autoptr(virJSONValue) reply = NULL;
+
 if (!cmd)
 return -1;
 
@@ -1630,8 +1624,6 @@ int qemuMonitorJSONSystemPowerdown(qemuMonitor *mon)
 
 ret = 0;
  cleanup:
-virJSONValueFree(cmd);
-virJSONValueFree(reply);
 return ret;
 }
 
@@ -1640,11 +1632,11 @@ int qemuMonitorJSONSetLink(qemuMonitor *mon,
virDomainNetInterfaceLinkState state)
 {
 int ret = -1;
-virJSONValue *reply = NULL;
-virJSONValue *cmd = qemuMonitorJSONMakeCommand("set_link",
- "s:name", name,
- "b:up", state != 
VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN,
- NULL);
+g_autoptr(virJSONValue) reply = NULL;
+g_autoptr(virJSONValue) cmd = qemuMonitorJSONMakeCommand("set_link",
+ "s:name", name,
+ "b:up", state != 
VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN,
+ NULL);
 
 if (!cmd)
 return -1;
@@ -1657,16 +1649,15 @@ int qemuMonitorJSONSetLink(qemuMonitor *mon,
 
 ret = 0;
  cleanup:
-virJSONValueFree(cmd);
-virJSONValueFree(reply);
 return ret;
 }
 
 int qemuMonitorJSONSystemReset(qemuMonitor *mon)
 {
 int ret = -1;
-virJSONValue *cmd = qemuMonitorJSONMakeCommand("system_reset", NULL);
-

[PATCH 13/13] qemu_monitor_json: Drop pointless error labels

[Michal Privoznik]

After previous cleanups, some 'error' labels were rendered
needless - they contain nothing more than a return statement.
Well, those labels can be dropped and 'goto error' can be
replaced with return statement directly.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_monitor_json.c | 25 -
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index bb76ac7d37..e9be9bdabd 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5351,7 +5351,7 @@ qemuMonitorJSONMakeCPUModel(virCPUDef *cpu,
 size_t i;
 
 if (virJSONValueObjectAppendString(model, "name", cpu->model) < 0)
-goto error;
+return NULL;
 
 if (cpu->nfeatures || !migratable) {
 g_autoptr(virJSONValue) props = virJSONValueNewObject();
@@ -5367,22 +5367,19 @@ qemuMonitorJSONMakeCPUModel(virCPUDef *cpu,
 enabled = true;
 
 if (virJSONValueObjectAppendBoolean(props, name, enabled) < 0)
-goto error;
+return NULL;
 }
 
 if (!migratable &&
 virJSONValueObjectAppendBoolean(props, "migratable", false) < 0) {
-goto error;
+return NULL;
 }
 
 if (virJSONValueObjectAppend(model, "props", ) < 0)
-goto error;
+return NULL;
 }
 
 return g_steal_pointer();
-
- error:
-return NULL;
 }
 
 
@@ -6930,20 +6927,17 @@ qemuMonitorJSONParseCPUx86Features(virJSONValue *data)
 size_t i;
 
 if (!(cpudata = virCPUDataNew(VIR_ARCH_X86_64)))
-goto error;
+return NULL;
 
 item.type = VIR_CPU_X86_DATA_CPUID;
 for (i = 0; i < virJSONValueArraySize(data); i++) {
 if (qemuMonitorJSONParseCPUx86FeatureWord(virJSONValueArrayGet(data, 
i),
   ) < 0 ||
 virCPUx86DataAdd(cpudata, ) < 0)
-goto error;
+return NULL;
 }
 
 return g_steal_pointer();
-
- error:
-return NULL;
 }
 
 
@@ -7049,20 +7043,17 @@ qemuMonitorJSONGetGuestCPUx86(qemuMonitor *mon,
 
 if (qemuMonitorJSONGetCPUx86Data(mon, "feature-words",
  ) < 0)
-goto error;
+return -1;
 
 if (disabled &&
 qemuMonitorJSONGetCPUx86Data(mon, "filtered-features",
  ) < 0)
-goto error;
+return -1;
 
 *data = g_steal_pointer();
 if (disabled)
 *disabled = g_steal_pointer();
 return 0;
-
- error:
-return -1;
 }
 
 
-- 
2.32.0

[PATCH 07/13] qemu_monitor_json: Use g_autoptr() for virCPUData

[Michal Privoznik]

We have g_autoptr() for virCPUData struct defined already. Let's
use it in qemu_monitor_json.c and drop explicit free calls.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_monitor_json.c | 21 -
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 655d2a022f..3d89afa6c6 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -7288,7 +7288,7 @@ qemuMonitorJSONParseCPUx86FeatureWord(virJSONValue *data,
 static virCPUData *
 qemuMonitorJSONParseCPUx86Features(virJSONValue *data)
 {
-virCPUData *cpudata = NULL;
+g_autoptr(virCPUData) cpudata = NULL;
 virCPUx86DataItem item = { 0 };
 size_t i;
 
@@ -7303,10 +7303,9 @@ qemuMonitorJSONParseCPUx86Features(virJSONValue *data)
 goto error;
 }
 
-return cpudata;
+return g_steal_pointer();
 
  error:
-virCPUDataFree(cpudata);
 return NULL;
 }
 
@@ -7418,8 +7417,8 @@ qemuMonitorJSONGetGuestCPUx86(qemuMonitor *mon,
   virCPUData **data,
   virCPUData **disabled)
 {
-virCPUData *cpuEnabled = NULL;
-virCPUData *cpuDisabled = NULL;
+g_autoptr(virCPUData) cpuEnabled = NULL;
+g_autoptr(virCPUData) cpuDisabled = NULL;
 int rc;
 
 if ((rc = qemuMonitorJSONCheckCPUx86(mon)) < 0)
@@ -7436,14 +7435,12 @@ qemuMonitorJSONGetGuestCPUx86(qemuMonitor *mon,
  ) < 0)
 goto error;
 
-*data = cpuEnabled;
+*data = g_steal_pointer();
 if (disabled)
-*disabled = cpuDisabled;
+*disabled = g_steal_pointer();
 return 0;
 
  error:
-virCPUDataFree(cpuEnabled);
-virCPUDataFree(cpuDisabled);
 return -1;
 }
 
@@ -7554,8 +7551,8 @@ qemuMonitorJSONGetGuestCPU(qemuMonitor *mon,
virCPUData **enabled,
virCPUData **disabled)
 {
-virCPUData *cpuEnabled = NULL;
-virCPUData *cpuDisabled = NULL;
+g_autoptr(virCPUData) cpuEnabled = NULL;
+g_autoptr(virCPUData) cpuDisabled = NULL;
 int ret = -1;
 
 if (!(cpuEnabled = virCPUDataNew(arch)) ||
@@ -7576,8 +7573,6 @@ qemuMonitorJSONGetGuestCPU(qemuMonitor *mon,
 ret = 0;
 
  cleanup:
-virCPUDataFree(cpuEnabled);
-virCPUDataFree(cpuDisabled);
 return ret;
 }
 
-- 
2.32.0

[PATCH 06/13] qemu_monitor: Declare and use g_autoptr for qemuMonitorEventPanicInfo

[Michal Privoznik]

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_monitor.h  |  1 +
 src/qemu/qemu_monitor_json.c | 22 ++
 2 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 2508e89503..8214c2fd9f 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -194,6 +194,7 @@ struct _qemuMonitorJobInfo {
 
 char *qemuMonitorGuestPanicEventInfoFormatMsg(qemuMonitorEventPanicInfo *info);
 void qemuMonitorEventPanicInfoFree(qemuMonitorEventPanicInfo *info);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuMonitorEventPanicInfo, 
qemuMonitorEventPanicInfoFree);
 void qemuMonitorEventRdmaGidStatusFree(qemuMonitorRdmaGidStatus *info);
 void qemuMonitorMemoryDeviceSizeChangeFree(qemuMonitorMemoryDeviceSizeChange 
*info);
 
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 26cbb8cedc..655d2a022f 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -605,9 +605,7 @@ static void qemuMonitorJSONHandleResume(qemuMonitor *mon, 
virJSONValue *data G_G
 static qemuMonitorEventPanicInfo *
 qemuMonitorJSONGuestPanicExtractInfoHyperv(virJSONValue *data)
 {
-qemuMonitorEventPanicInfo *ret;
-
-ret = g_new0(qemuMonitorEventPanicInfo, 1);
+g_autoptr(qemuMonitorEventPanicInfo) ret = 
g_new0(qemuMonitorEventPanicInfo, 1);
 
 ret->type = QEMU_MONITOR_EVENT_PANIC_INFO_TYPE_HYPERV;
 
@@ -618,20 +616,16 @@ qemuMonitorJSONGuestPanicExtractInfoHyperv(virJSONValue 
*data)
 virJSONValueObjectGetNumberUlong(data, "arg5", >data.hyperv.arg5) 
< 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("malformed hyperv panic data"));
-goto error;
+return NULL;
 }
 
-return ret;
-
- error:
-qemuMonitorEventPanicInfoFree(ret);
-return NULL;
+return g_steal_pointer();
 }
 
 static qemuMonitorEventPanicInfo *
 qemuMonitorJSONGuestPanicExtractInfoS390(virJSONValue *data)
 {
-qemuMonitorEventPanicInfo *ret;
+g_autoptr(qemuMonitorEventPanicInfo) ret = NULL;
 int core;
 unsigned long long psw_mask, psw_addr;
 const char *reason = NULL;
@@ -645,7 +639,7 @@ qemuMonitorJSONGuestPanicExtractInfoS390(virJSONValue *data)
 virJSONValueObjectGetNumberUlong(data, "psw-addr", _addr) < 0 ||
 !(reason = virJSONValueObjectGetString(data, "reason"))) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed s390 panic 
data"));
-goto error;
+return NULL;
 }
 
 ret->data.s390.core = core;
@@ -654,11 +648,7 @@ qemuMonitorJSONGuestPanicExtractInfoS390(virJSONValue 
*data)
 
 ret->data.s390.reason = g_strdup(reason);
 
-return ret;
-
- error:
-qemuMonitorEventPanicInfoFree(ret);
-return NULL;
+return g_steal_pointer();
 }
 
 static qemuMonitorEventPanicInfo *
-- 
2.32.0

[PATCH 04/13] qemuMonitorJSONGetMigrationStats: Don't clear @stats on failure

[Michal Privoznik]

In the qemuMonitorJSONGetMigrationStats() there's a code under
cleanup label that's clearing returned @stats if the function
returns with an error. However, transitively there's just one
caller - qemuMigrationAnyFetchStats() - and it doesn't care for
this behaviour. Drop the code to simplify the cleanup label.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_monitor_json.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 7833038a06..bcbb4e59ab 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -3542,8 +3542,6 @@ int qemuMonitorJSONGetMigrationStats(qemuMonitor *mon,
 
 ret = 0;
  cleanup:
-if (ret < 0)
-memset(stats, 0, sizeof(*stats));
 virJSONValueFree(cmd);
 virJSONValueFree(reply);
 return ret;
-- 
2.32.0

[PATCH 05/13] qemuMonitorJSONQueryRxFilterParse: Set *filter only on success

[Michal Privoznik]

The qemuMonitorJSONQueryRxFilterParse() function is called to
parse the output of 'query-rx-filter' and store results into
passed virNetDevRxFilter structure. However, it is doing so in a
bit clumsy way - the return pointer is set in all cases (i.e.
even in case of error) and thus the cleanup label is more
complicated than it needs to be. With a help of g_autoptr() and
g_steal_pointer() the return pointer can be set only in case of
success - which is what callers expect anyway.

The same applies to qemuMonitorJSONQueryRxFilter().

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_monitor_json.c | 12 ++--
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index bcbb4e59ab..26cbb8cedc 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4035,7 +4035,7 @@ qemuMonitorJSONQueryRxFilterParse(virJSONValue *msg,
 virJSONValue *element;
 size_t nTable;
 size_t i;
-virNetDevRxFilter *fil = virNetDevRxFilterNew();
+g_autoptr(virNetDevRxFilter) fil = virNetDevRxFilterNew();
 
 if (!fil)
 goto cleanup;
@@ -4187,13 +4187,9 @@ qemuMonitorJSONQueryRxFilterParse(virJSONValue *msg,
 }
 fil->vlan.nTable = nTable;
 
+*filter = g_steal_pointer();
 ret = 0;
  cleanup:
-if (ret < 0) {
-virNetDevRxFilterFree(fil);
-fil = NULL;
-}
-*filter = fil;
 return ret;
 }
 
@@ -4222,10 +4218,6 @@ qemuMonitorJSONQueryRxFilter(qemuMonitor *mon, const 
char *alias,
 
 ret = 0;
  cleanup:
-if (ret < 0) {
-virNetDevRxFilterFree(*filter);
-*filter = NULL;
-}
 virJSONValueFree(cmd);
 virJSONValueFree(reply);
 return ret;
-- 
2.32.0

[PATCH 01/13] qemu_monitor_json: Don't check for qemuMonitorNextCommandID() retval

[Michal Privoznik]

The qemuMonitorNextCommandID() function can never fail. There's
no need to check for its retval then. Moreover, the temporary
variable used to hold the retval can be declared in the inner
most block.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_monitor_json.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index a7a980fccd..dcf9186191 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -302,15 +302,14 @@ qemuMonitorJSONCommandWithFd(qemuMonitor *mon,
 int ret = -1;
 qemuMonitorMessage msg;
 g_auto(virBuffer) cmdbuf = VIR_BUFFER_INITIALIZER;
-char *id = NULL;
 
 *reply = NULL;
 
 memset(, 0, sizeof(msg));
 
 if (virJSONValueObjectHasKey(cmd, "execute") == 1) {
-if (!(id = qemuMonitorNextCommandID(mon)))
-goto cleanup;
+g_autofree char *id = qemuMonitorNextCommandID(mon);
+
 if (virJSONValueObjectAppendString(cmd, "id", id) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Unable to append command 'id' string"));
@@ -339,7 +338,6 @@ qemuMonitorJSONCommandWithFd(qemuMonitor *mon,
 }
 
  cleanup:
-VIR_FREE(id);
 VIR_FREE(msg.txBuffer);
 
 return ret;
-- 
2.32.0

[PATCH 03/13] qemuMonitorJSONHumanCommand: Require @reply_str

[Michal Privoznik]

All callers of qemuMonitorJSONHumanCommand() pass a non-NULL pointer
as @reply_str therefore there's no need to check whether it is NULL.
NB, the sister function (qemuMonitorJSONArbitraryCommand()) doesn't
check for NULL either.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_monitor_json.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 6d8ccd91e8..7833038a06 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1470,6 +1470,7 @@ qemuMonitorJSONHumanCommand(qemuMonitor *mon,
 virJSONValue *cmd = NULL;
 virJSONValue *reply = NULL;
 virJSONValue *obj;
+const char *data;
 int ret = -1;
 
 cmd = qemuMonitorJSONMakeCommand("human-monitor-command",
@@ -1490,13 +1491,8 @@ qemuMonitorJSONHumanCommand(qemuMonitor *mon,
 goto cleanup;
 
 obj = virJSONValueObjectGet(reply, "return");
-
-if (reply_str) {
-const char *data;
-
-data = virJSONValueGetString(obj);
-*reply_str = g_strdup(NULLSTR_EMPTY(data));
-}
+data = virJSONValueGetString(obj);
+*reply_str = g_strdup(NULLSTR_EMPTY(data));
 
 ret = 0;
 
-- 
2.32.0

[PATCH 02/13] qemu_monitor_json: Don't transfer ownership to @msg

[Michal Privoznik]

In qemuMonitorJSONCommandWithFd() given command (represented by
virJSONValue struct) is translated to string (represented by
virBuffer). The ownership of the string is then transferred to
the message which is then sent. The downside of this approach is
we have to have an explicit call to free the string from the
message. But if the message just "borrowed" the string (which it
can safely do because it is just reading from the string) then
automatic free of the buffer takes care of freeing the string.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_monitor.c  | 2 +-
 src/qemu/qemu_monitor.h  | 2 +-
 src/qemu/qemu_monitor_json.c | 4 +---
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 81d9087839..908ee0d302 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -420,7 +420,7 @@ static int
 qemuMonitorIOWrite(qemuMonitor *mon)
 {
 int done;
-char *buf;
+const char *buf;
 size_t len;
 
 /* If no active message, or fully transmitted, the no-op */
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index f452d0d306..2508e89503 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -39,7 +39,7 @@ typedef struct _qemuMonitorMessage qemuMonitorMessage;
 struct _qemuMonitorMessage {
 int txFD;
 
-char *txBuffer;
+const char *txBuffer;
 int txOffset;
 int txLength;
 
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index dcf9186191..6d8ccd91e8 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -322,7 +322,7 @@ qemuMonitorJSONCommandWithFd(qemuMonitor *mon,
 virBufferAddLit(, "\r\n");
 
 msg.txLength = virBufferUse();
-msg.txBuffer = virBufferContentAndReset();
+msg.txBuffer = virBufferCurrentContent();
 msg.txFD = scm_fd;
 
 ret = qemuMonitorSend(mon, );
@@ -338,8 +338,6 @@ qemuMonitorJSONCommandWithFd(qemuMonitor *mon,
 }
 
  cleanup:
-VIR_FREE(msg.txBuffer);
-
 return ret;
 }
 
-- 
2.32.0

[PATCH 00/13] qemu_monitor_json: Use g_auto* more

[Michal Privoznik]

*** BLURB HERE ***

Michal Prívozník (13):
  qemu_monitor_json: Don't check for qemuMonitorNextCommandID() retval
  qemu_monitor_json: Don't transfer ownership to @msg
  qemuMonitorJSONHumanCommand: Require @reply_str
  qemuMonitorJSONGetMigrationStats: Don't clear @stats on failure
  qemuMonitorJSONQueryRxFilterParse: Set *filter only on success
  qemu_monitor: Declare and use g_autoptr for qemuMonitorEventPanicInfo
  qemu_monitor_json: Use g_autoptr() for virCPUData
  qemu_monitor_json: Use g_autoptr() for qemuMonitorCPUModelInfo
  qemuMonitorJSONExtractPRManagerInfo: Declare @entry inside the loop
  qemu_monitor_json: Use g_autoptr() for virJSONValue
  qemu_monitor_json: Use g_autofree
  qemu_monitor_json: Drop pointless cleanup labels
  qemu_monitor_json: Drop pointless error labels

 src/qemu/qemu_monitor.c  |2 +-
 src/qemu/qemu_monitor.h  |3 +-
 src/qemu/qemu_monitor_json.c | 1803 --
 3 files changed, 641 insertions(+), 1167 deletions(-)

-- 
2.32.0

Re: [PATCH] lib: Drop intermediary return variables

[Ján Tomko]

On a Friday in 2021, Michal Privoznik wrote:

In a few places we declare a variable (which is optionally
followed by a code not touching it) then set the variable to a
value and return the variable immediately. It's obvious that the
variable is needless and the value can be returned directly
instead.

This patch was generated using this semantic patch:

 @@
 type T;
 identifier ret;
 expression E;
 @@
 - T ret;
 ... when != ret
 when strict
 - ret = E;
 - return ret;
 + return E;

After that I fixed couple of formatting issues because coccinelle
formatted some lines differently than our coding style.

Signed-off-by: Michal Privoznik 
---
src/bhyve/bhyve_domain.c|  6 +-
src/conf/domain_addr.c  |  6 +-
src/conf/domain_conf.c  |  5 +
src/conf/virnetworkobj.c|  4 +---
src/conf/virnwfilterbindingobj.c|  5 +
src/conf/virnwfilterobj.c   |  5 +
src/esx/esx_driver.c|  5 +
src/esx/esx_storage_backend_iscsi.c |  5 +
src/esx/esx_storage_backend_vmfs.c  |  5 +
src/hyperv/hyperv_driver.c  |  5 +
src/locking/lock_daemon.c   |  5 +
src/locking/lock_driver_sanlock.c   |  6 ++
src/logging/log_daemon.c|  5 +
src/network/bridge_driver.c | 27 +--
src/qemu/qemu_alias.c   | 24 
src/qemu/qemu_command.c | 15 ++-
src/qemu/qemu_domain.c  | 10 +++---
src/qemu/qemu_monitor.c |  5 +
src/security/security_apparmor.c|  5 +
src/security/security_nop.c |  5 +
src/test/test_driver.c  |  9 ++---
src/util/viraudit.c |  4 +---
src/util/virfirewall.c  |  6 +-
src/util/virmacmap.c|  6 +-
src/util/virnetdev.c|  5 +
src/util/virpci.c   | 10 ++
src/vbox/vbox_common.c  |  7 +++
tests/bhyvexml2argvmock.c   |  5 +
tests/qemumonitortestutils.c|  5 +
tests/qemusecuritymock.c|  6 +-
tests/qemuxml2argvmock.c|  5 +
tests/virnetserverclienttest.c  |  6 +-
tests/virpcimock.c  |  8 ++--
tests/virusbmock.c  |  4 +---
34 files changed, 54 insertions(+), 190 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

Re: [PATCH v3 0/4] Add qemu support setting qos via ovs on ovs interface

[Yalan Zhang]

Hi Jinsheng,

I have tested the patch and have some questions, could you please help to
confirm?
1) For inbound, how to check it from the openvswitch side? tc will still
show the statistics, is that expected?
2) For outbound, the peak is ignored. I just can not understand the
"ingress_policing_burst:
2048", how can it come from the setting "outbound.burst : 256"?
3) Is the output from tc command expected?

Test inbound:
1. start vm with setting as below:
 
  
  





  

...


2.

# virsh domiftune rhel vnet5

inbound.average: 100

inbound.peak   : 200

inbound.burst  : 256

inbound.floor  : 0

outbound.average: 0

outbound.peak  : 0

outbound.burst : 0

# ip l

17: vnet5:  mtu 1500 qdisc htb master
ovs-system state UNKNOWN mode DEFAULT group default qlen 1000

link/ether fe:54:00:4d:43:5a brd ff:ff:ff:ff:ff:ff

# ovs-vsctl show interface

…...

ingress_policing_burst: 0

ingress_policing_kpkts_burst: 0

ingress_policing_kpkts_rate: 0

ingress_policing_rate: 0

…...

name: vnet5

#  tc -d class show  dev vnet5

class htb 1:1 parent 1:fffe prio 0 quantum 10240 rate 819200bit ceil
1638Kbit linklayer ethernet burst 256Kb/1 mpu 0b cburst 256Kb/1 mpu 0b
level 0

class htb 1:fffe root rate 1638Kbit ceil 1638Kbit linklayer ethernet burst
1499b/1 mpu 0b cburst 1499b/1 mpu 0b level 7

#  tc -d filter show dev vnet5 parent :
(no outputs)

For outbound:

# virsh dumpxml rhel | grep /bandwidth -B2

 



  

# virsh domiftune rhel vnet9

inbound.average: 0

inbound.peak   : 0

inbound.burst  : 0

inbound.floor  : 0

outbound.average: 100

outbound.peak  : 200

outbound.burst : 256

# ovs-vsctl list interface

ingress_policing_burst: *2048*

ingress_policing_kpkts_burst: 0

ingress_policing_kpkts_rate: 0

ingress_policing_rate: *800*
...

# tc -d filter show dev vnet9 parent :

filter protocol all pref 49 basic chain 0

filter protocol all pref 49 basic chain 0 handle 0x1

action order 1:  police 0x1 rate 800Kbit burst 256Kb mtu 64Kb action
drop/pipe overhead 0b linklayer unspec

ref 1 bind 1

# tc -d class show  dev vnet9

(no outputs)


---
Best Regards,
Yalan Zhang
IRC: yalzhang


On Mon, Jul 12, 2021 at 3:43 PM Michal Prívozník 
wrote:

> On 7/9/21 3:31 PM, Jinsheng Zhang (张金生)-云服务集团 wrote:
> > Here is my signed-off-by line
> >
> > Signed-off-by: zhangj...@inspur.com
> >
> > Thanks again for reminding:) .
>
> Perfect.
>
> Reviewed-by: Michal Privoznik 
>
> and pushed. Congratulations on your first libvirt contribution!
>
> Michal
>
>

[PATCH v1 1/2] qemu_process: set fakereboot flags false after processing fakereboot over

[Bihong Yu]

During the vm rebooting, the vm could be shut down if the libvirtd is
restarted for some reason, which is not expected. We move set
fakereboot flags false after processing fakereboot over, so we can
ensure that fakereboot process have been executed.

Signed-off-by: Bihong Yu 
---
 src/qemu/qemu_process.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 6027b30405..832ce164fb 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -523,6 +523,7 @@ qemuProcessFakeReboot(void *opaque)
 
  cleanup:
 priv->pausedShutdown = false;
+qemuDomainSetFakeReboot(driver, vm, false);
 if (ret == -1)
 ignore_value(qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_FORCE));
 virDomainObjEndAPI();
@@ -540,7 +541,6 @@ qemuProcessShutdownOrReboot(virQEMUDriver *driver,
 g_autofree char *name = g_strdup_printf("reboot-%s", vm->def->name);
 virThread th;
 
-qemuDomainSetFakeReboot(driver, vm, false);
 virObjectRef(vm);
 if (virThreadCreateFull(,
 false,
@@ -551,6 +551,7 @@ qemuProcessShutdownOrReboot(virQEMUDriver *driver,
 VIR_ERROR(_("Failed to create reboot thread, killing domain"));
 ignore_value(qemuProcessKill(vm, VIR_QEMU_PROCESS_KILL_NOWAIT));
 priv->pausedShutdown = false;
+qemuDomainSetFakeReboot(driver, vm, false);
 virObjectUnref(vm);
 }
 } else {
-- 
2.27.0

[PATCH v1 2/2] qemu_process: continue to process fakereboot after restarting libvirtd

[Bihong Yu]

During the vm rebooting, the vm could be paused if the libvirtd is
restarted for some reason, which is not expected. We need continue
fakereboot process if fakereboot flags is true and the vm is in
paused-user status.

Signed-off-by: Bihong Yu 
---
 src/qemu/qemu_process.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 832ce164fb..a758b96fa6 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8742,13 +8742,15 @@ qemuProcessReconnect(void *opaque)
 goto error;
 }
 
-/* In case the domain shutdown while we were not running,
- * we need to finish the shutdown process. And we need to do it after
- * we have virQEMUCaps filled in.
+/* In case the domain shutdown or fake reboot while we were not running,
+ * we need to finish the shutdown or fake reboot process. And we need to
+ * do it after we have virQEMUCaps filled in.
  */
 if (state == VIR_DOMAIN_SHUTDOWN ||
 (state == VIR_DOMAIN_PAUSED &&
- reason == VIR_DOMAIN_PAUSED_SHUTTING_DOWN)) {
+ reason == VIR_DOMAIN_PAUSED_SHUTTING_DOWN) ||
+(priv->fakeReboot && state == VIR_DOMAIN_PAUSED &&
+ reason == VIR_DOMAIN_PAUSED_USER)) {
 VIR_DEBUG("Finishing shutdown sequence for domain %s",
   obj->def->name);
 qemuProcessShutdownOrReboot(driver, obj);
-- 
2.27.0

[PATCH v1 0/2] qemu_process: ensure the reboot process is performed completely

[Bihong Yu]

When the vm reboot in ACPI mode, the vm has a certain probability to be shutoff
or paused status if the libvirtd is restarted for some reason, which is not
expected.

This patchset ensure the reboot process is performed completely.

Bihong Yu (2):
  qemu_process: set fakereboot flags false after processing fakereboot
over
  qemu_process: continue to process fakereboot after restarting libvirtd

 src/qemu/qemu_process.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

-- 
2.27.0

Re: [PATCH 2/9] qapi: Mark unstable QMP parts with feature 'unstable'

[Juan Quintela]

Markus Armbruster  wrote:
> Add special feature 'unstable' everywhere the name starts with 'x-',
> except for InputBarrierProperties member x-origin and
> MemoryBackendProperties member x-use-canonical-path-for-ramblock-id,
> because these two are actually stable.
>
> Signed-off-by: Markus Armbruster 

Reviewed-by: Juan Quintela 

[libvirt PATCH 09/13] ch_driver, ch_domain: vcpupin callback in ch driver

[Vineeth Pillai]

Signed-off-by: Vineeth Pillai 
Signed-off-by: Praveen K Paladugu 
---
 src/ch/ch_domain.c |  30 +
 src/ch/ch_domain.h |   1 +
 src/ch/ch_driver.c | 151 +
 3 files changed, 182 insertions(+)

diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c
index d0aaeed1f4..9ad00583aa 100644
--- a/src/ch/ch_domain.c
+++ b/src/ch/ch_domain.c
@@ -20,6 +20,7 @@
 
 #include 
 
+#include "datatypes.h"
 #include "ch_domain.h"
 #include "domain_driver.h"
 #include "viralloc.h"
@@ -420,3 +421,32 @@ char *virCHDomainGetMachineName(virDomainObj *vm)
 
 return ret;
 }
+
+/**
+ * virCHDomainObjFromDomain:
+ * @domain: Domain pointer that has to be looked up
+ *
+ * This function looks up @domain and returns the appropriate virDomainObjPtr
+ * that has to be released by calling virDomainObjEndAPI().
+ *
+ * Returns the domain object with incremented reference counter which is locked
+ * on success, NULL otherwise.
+ */
+virDomainObj *
+virCHDomainObjFromDomain(virDomainPtr domain)
+{
+virDomainObj *vm;
+virCHDriver *driver = domain->conn->privateData;
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+vm = virDomainObjListFindByUUID(driver->domains, domain->uuid);
+if (!vm) {
+virUUIDFormat(domain->uuid, uuidstr);
+virReportError(VIR_ERR_NO_DOMAIN,
+   _("no domain with matching uuid '%s' (%s)"),
+   uuidstr, domain->name);
+return NULL;
+}
+
+return vm;
+}
diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h
index 2ce3e2cef3..db1451405b 100644
--- a/src/ch/ch_domain.h
+++ b/src/ch/ch_domain.h
@@ -95,3 +95,4 @@ pid_t virCHDomainGetVcpuPid(virDomainObj *vm, unsigned int 
vcpuid);
 bool virCHDomainHasVcpuPids(virDomainObj *vm);
 
 char *virCHDomainGetMachineName(virDomainObj *vm);
+virDomainObj *virCHDomainObjFromDomain(virDomain *domain);
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index ca854da123..7f3cf6dbef 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -25,6 +25,7 @@
 #include "ch_driver.h"
 #include "ch_monitor.h"
 #include "ch_process.h"
+#include "ch_cgroup.h"
 #include "datatypes.h"
 #include "driver.h"
 #include "viraccessapicheck.h"
@@ -1141,6 +1142,154 @@ chDomainGetVcpus(virDomainPtr dom,
 return ret;
 }
 
+static int
+chDomainPinVcpuLive(virDomainObj *vm,
+virDomainDef *def,
+int vcpu,
+virCHDriver *driver,
+virCHDriverConfig *cfg,
+virBitmap *cpumap)
+{
+virBitmap *tmpmap = NULL;
+virDomainVcpuDef *vcpuinfo;
+virCHDomainObjPrivate *priv = vm->privateData;
+virCgroup *cgroup_vcpu = NULL;
+g_autofree char *str = NULL;
+int ret = -1;
+
+if (!virCHDomainHasVcpuPids(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   "%s", _("cpu affinity is not supported"));
+goto cleanup;
+}
+
+if (!(vcpuinfo = virDomainDefGetVcpu(def, vcpu))) {
+virReportError(VIR_ERR_INVALID_ARG,
+   _("vcpu %d is out of range of live cpu count %d"),
+   vcpu, virDomainDefGetVcpusMax(def));
+goto cleanup;
+}
+
+if (!(tmpmap = virBitmapNewCopy(cpumap)))
+goto cleanup;
+
+if (!(str = virBitmapFormat(cpumap)))
+goto cleanup;
+
+if (vcpuinfo->online) {
+/* Configure the corresponding cpuset cgroup before set affinity. */
+if (virCgroupHasController(priv->cgroup, 
VIR_CGROUP_CONTROLLER_CPUSET)) {
+if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, vcpu,
+   false, _vcpu) < 0)
+goto cleanup;
+if (chSetupCgroupCpusetCpus(cgroup_vcpu, cpumap) < 0)
+goto cleanup;
+}
+
+if (virProcessSetAffinity(virCHDomainGetVcpuPid(vm, vcpu), cpumap, 
false) < 0)
+goto cleanup;
+}
+
+virBitmapFree(vcpuinfo->cpumask);
+vcpuinfo->cpumask = tmpmap;
+tmpmap = NULL;
+
+if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0)
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+virBitmapFree(tmpmap);
+virCgroupFree(cgroup_vcpu);
+return ret;
+}
+
+
+static int
+chDomainPinVcpuFlags(virDomainPtr dom,
+ unsigned int vcpu,
+ unsigned char *cpumap,
+ int maplen,
+ unsigned int flags)
+{
+virCHDriver *driver = dom->conn->privateData;
+virDomainObj *vm;
+virDomainDef *def;
+virDomainDef *persistentDef;
+int ret = -1;
+virBitmap *pcpumap = NULL;
+virDomainVcpuDef *vcpuinfo = NULL;
+g_autoptr(virCHDriverConfig) cfg = NULL;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+cfg = virCHDriverGetConfig(driver);
+
+if (!(vm = virCHDomainObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainPinVcpuFlagsEnsureACL(dom->conn, 

[libvirt PATCH 03/13] ch_domain: add virCHDomainGetMonitor helper method

[Vineeth Pillai]

Signed-off-by: Vineeth Pillai 
Signed-off-by: Praveen K Paladugu 
---
 src/ch/ch_domain.c | 6 ++
 src/ch/ch_domain.h | 5 +
 2 files changed, 11 insertions(+)

diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c
index 9d32d8669a..ae79b47253 100644
--- a/src/ch/ch_domain.c
+++ b/src/ch/ch_domain.c
@@ -292,3 +292,9 @@ virDomainDefParserConfig virCHDriverDomainDefParserConfig = 
{
 .domainPostParseCallback = virCHDomainDefPostParse,
 .deviceValidateCallback = chValidateDomainDeviceDef,
 };
+
+virCHMonitor *
+virCHDomainGetMonitor(virDomainObj *vm)
+{
+return CH_DOMAIN_PRIVATE(vm)->monitor;
+}
diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h
index 61b34b0467..04d19398b4 100644
--- a/src/ch/ch_domain.h
+++ b/src/ch/ch_domain.h
@@ -57,6 +57,11 @@ struct _virCHDomainObjPrivate {
  virChrdevs *chrdevs;
 };
 
+#define CH_DOMAIN_PRIVATE(vm) \
+((virCHDomainObjPrivate *) (vm)->privateData)
+
+virCHMonitor *virCHDomainGetMonitor(virDomainObj *vm);
+
 extern virDomainXMLPrivateDataCallbacks virCHDriverPrivateDataCallbacks;
 extern virDomainDefParserConfig virCHDriverDomainDefParserConfig;
 
-- 
2.27.0

[libvirt PATCH 10/13] ch_driver: enable typed param string for numatune

[Vineeth Pillai]

Enable support of VIR_DRV_FEATURE_TYPED_PARAM_STRING to enable numatune

Signed-off-by: Vineeth Pillai 
Signed-off-by: Praveen K Paladugu 
---
 src/ch/ch_driver.c | 31 +++
 1 file changed, 31 insertions(+)

diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 7f3cf6dbef..9ad23c1710 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -955,6 +955,36 @@ static int chStateInitialize(bool privileged,
 return ret;
 }
 
+/* Which features are supported by this driver? */
+static int
+chConnectSupportsFeature(virConnectPtr conn, int feature)
+{
+if (virConnectSupportsFeatureEnsureACL(conn) < 0)
+return -1;
+
+switch ((virDrvFeature) feature) {
+case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
+return 1;
+case VIR_DRV_FEATURE_MIGRATION_V2:
+case VIR_DRV_FEATURE_MIGRATION_V3:
+case VIR_DRV_FEATURE_MIGRATION_P2P:
+case VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION:
+case VIR_DRV_FEATURE_FD_PASSING:
+case VIR_DRV_FEATURE_XML_MIGRATABLE:
+case VIR_DRV_FEATURE_MIGRATION_OFFLINE:
+case VIR_DRV_FEATURE_MIGRATION_PARAMS:
+case VIR_DRV_FEATURE_MIGRATION_DIRECT:
+case VIR_DRV_FEATURE_MIGRATION_V1:
+case VIR_DRV_FEATURE_PROGRAM_KEEPALIVE:
+case VIR_DRV_FEATURE_REMOTE:
+case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK:
+case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK:
+case VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER:
+default:
+return 0;
+}
+}
+
 static int
 chDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags)
 {
@@ -1303,6 +1333,7 @@ static virHypervisorDriver chHypervisorDriver = {
 .connectListAllDomains = chConnectListAllDomains,   /* 7.5.0 */
 .connectListDomains = chConnectListDomains, /* 7.5.0 */
 .connectGetCapabilities = chConnectGetCapabilities, /* 7.5.0 */
+.connectSupportsFeature = chConnectSupportsFeature, /* 7.8.0 */
 .domainCreateXML = chDomainCreateXML,   /* 7.5.0 */
 .domainCreate = chDomainCreate, /* 7.5.0 */
 .domainCreateWithFlags = chDomainCreateWithFlags,   /* 7.5.0 */
-- 
2.27.0

Re: [PATCH 1/9] qapi: New special feature flag "unstable"

[Juan Quintela]

Markus Armbruster  wrote:
> By convention, names starting with "x-" are experimental.  The parts
> of external interfaces so named may be withdrawn or changed
> incompatibly in future releases.
>
> Drawback: promoting something from experimental to stable involves a
> name change.  Client code needs to be updated.
>
> Moreover, the convention is not universally observed:
>
> * QOM type "input-barrier" has properties "x-origin", "y-origin".
>   Looks accidental, but it's ABI since 4.2.
>
> * QOM types "memory-backend-file", "memory-backend-memfd",
>   "memory-backend-ram", and "memory-backend-epc" have a property
>   "x-use-canonical-path-for-ramblock-id" that is documented to be
>   stable despite its name.
>
> We could document these exceptions, but documentation helps only
> humans.  We want to recognize "unstable" in code, like "deprecated".
>
> Replace the convention by a new special feature flag "unstable".  It
> will be recognized by the QAPI generator, like the existing feature
> flag "deprecated", and unlike regular feature flags.
>
> This commit updates documentation and prepares tests.  The next commit
> updates the QAPI schema.  The remaining patches update the QAPI
> generator and wire up -compat policy checking.
>
> Signed-off-by: Markus Armbruster 

Reviewed-by: Juan Quintela 

[libvirt PATCH 08/13] ch_cgroup: methods for cgroup mgmt in ch driver

[Vineeth Pillai]

Signed-off-by: Vineeth Pillai 
Signed-off-by: Praveen K Paladugu 
---
 po/POTFILES.in  |   1 +
 src/ch/ch_cgroup.c  | 457 
 src/ch/ch_cgroup.h  |  45 +
 src/ch/ch_conf.c|   2 +
 src/ch/ch_conf.h|   4 +-
 src/ch/ch_domain.c  |  33 
 src/ch/ch_domain.h  |   3 +-
 src/ch/ch_monitor.c | 125 ++--
 src/ch/ch_monitor.h |  54 +-
 src/ch/ch_process.c | 288 +++-
 src/ch/ch_process.h |   3 +
 src/ch/meson.build  |   2 +
 12 files changed, 991 insertions(+), 26 deletions(-)
 create mode 100644 src/ch/ch_cgroup.c
 create mode 100644 src/ch/ch_cgroup.h

diff --git a/po/POTFILES.in b/po/POTFILES.in
index b554cf08ca..3a8db501bc 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -19,6 +19,7 @@
 @SRCDIR@src/bhyve/bhyve_parse_command.c
 @SRCDIR@src/bhyve/bhyve_process.c
 @SRCDIR@src/ch/ch_conf.c
+@SRCDIR@src/ch/ch_cgroup.c
 @SRCDIR@src/ch/ch_domain.c
 @SRCDIR@src/ch/ch_driver.c
 @SRCDIR@src/ch/ch_monitor.c
diff --git a/src/ch/ch_cgroup.c b/src/ch/ch_cgroup.c
new file mode 100644
index 00..6be2184cf1
--- /dev/null
+++ b/src/ch/ch_cgroup.c
@@ -0,0 +1,457 @@
+/*
+ * ch_cgroup.c: CH cgroup management
+ *
+ * Copyright Microsoft Corp. 2020-2021
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ */
+
+#include 
+
+#include "ch_cgroup.h"
+#include "ch_domain.h"
+#include "ch_process.h"
+#include "vircgroup.h"
+#include "virlog.h"
+#include "viralloc.h"
+#include "virerror.h"
+#include "domain_audit.h"
+#include "domain_cgroup.h"
+#include "virscsi.h"
+#include "virstring.h"
+#include "virfile.h"
+#include "virtypedparam.h"
+#include "virnuma.h"
+#include "virdevmapper.h"
+#include "virutil.h"
+
+#define VIR_FROM_THIS VIR_FROM_CH
+
+VIR_LOG_INIT("ch.ch_cgroup");
+
+static int
+chSetupBlkioCgroup(virDomainObj * vm)
+{
+virCHDomainObjPrivate *priv = vm->privateData;
+
+if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) {
+if (vm->def->blkio.weight || vm->def->blkio.ndevices) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("Block I/O tuning is not available on this 
host"));
+return -1;
+} else {
+return 0;
+}
+}
+
+return virDomainCgroupSetupBlkio(priv->cgroup, vm->def->blkio);
+}
+
+
+static int
+chSetupMemoryCgroup(virDomainObj * vm)
+{
+virCHDomainObjPrivate *priv = vm->privateData;
+
+if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_MEMORY)) {
+if (virMemoryLimitIsSet(vm->def->mem.hard_limit) ||
+virMemoryLimitIsSet(vm->def->mem.soft_limit) ||
+virMemoryLimitIsSet(vm->def->mem.swap_hard_limit)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("Memory cgroup is not available on this host"));
+return -1;
+} else {
+return 0;
+}
+}
+
+return virDomainCgroupSetupMemtune(priv->cgroup, vm->def->mem);
+}
+
+static int
+chSetupCpusetCgroup(virDomainObj * vm)
+{
+virCHDomainObjPrivate *priv = vm->privateData;
+
+if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET))
+return 0;
+
+if (virCgroupSetCpusetMemoryMigrate(priv->cgroup, true) < 0)
+return -1;
+
+return 0;
+}
+
+
+static int
+chSetupCpuCgroup(virDomainObj * vm)
+{
+virCHDomainObjPrivate *priv = vm->privateData;
+
+if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) {
+if (vm->def->cputune.sharesSpecified) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("CPU tuning is not available on this host"));
+return -1;
+} else {
+return 0;
+}
+}
+
+if (vm->def->cputune.sharesSpecified) {
+
+if (virCgroupSetCpuShares(priv->cgroup, vm->def->cputune.shares) < 0)
+return -1;
+
+}
+
+return 0;
+}
+
+
+static int
+chInitCgroup(virDomainObj * vm, size_t nnicindexes, int *nicindexes)
+{
+virCHDomainObjPrivate *priv = vm->privateData;
+
+g_autoptr(virCHDriverConfig) cfg = virCHDriverGetConfig(priv->driver);
+
+if (!priv->driver->privileged)
+return 0;
+
+if (!virCgroupAvailable())
+return 0;
+
+virCgroupFree(priv->cgroup);
+
+if (!vm->def->resource) 

[libvirt PATCH 01/13] util: Helper functions to get process info

[Vineeth Pillai]

These helper methods are used to capture vcpu information
in ch driver.

Signed-off-by: Vineeth Pillai 
Signed-off-by: Praveen K Paladugu 
---
 src/util/virprocess.c | 136 ++
 src/util/virprocess.h |   5 ++
 2 files changed, 141 insertions(+)

diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 6de3f36f52..0164d70df6 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -1721,3 +1721,139 @@ virProcessSetScheduler(pid_t pid G_GNUC_UNUSED,
 }
 
 #endif /* !WITH_SCHED_SETSCHEDULER */
+
+/*
+TODO: This method was cloned from qemuGetProcessInfo in src/qemu/qemu_driver.c.
+Need to refactor qemu driver to use this shared function.
+*/
+int
+virProcessGetStatInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss,
+   pid_t pid, pid_t tid)
+{
+g_autofree char *proc = NULL;
+FILE *pidinfo;
+unsigned long long usertime = 0, systime = 0;
+long rss = 0;
+int cpu = 0;
+
+/* In general, we cannot assume pid_t fits in int; but /proc parsing
+ * is specific to Linux where int works fine.  */
+if (tid)
+proc = g_strdup_printf("/proc/%d/task/%d/stat", (int)pid, tid);
+else
+proc = g_strdup_printf("/proc/%d/stat", (int)pid);
+if (!proc)
+return -1;
+
+pidinfo = fopen(proc, "r");
+
+/* See 'man proc' for information about what all these fields are. We're
+ * only interested in a very few of them */
+if (!pidinfo ||
+fscanf(pidinfo,
+   /* pid -> stime */
+   "%*d (%*[^)]) %*c %*d %*d %*d %*d %*d %*u %*u %*u %*u %*u %llu 
%llu"
+   /* cutime -> endcode */
+   "%*d %*d %*d %*d %*d %*d %*u %*u %ld %*u %*u %*u"
+   /* startstack -> processor */
+   "%*u %*u %*u %*u %*u %*u %*u %*u %*u %*u %*d %d",
+   , , , ) != 4) {
+VIR_WARN("cannot parse process status data");
+}
+
+/* We got jiffies
+ * We want nanoseconds
+ * _SC_CLK_TCK is jiffies per second
+ * So calculate thus
+ */
+if (cpuTime)
+*cpuTime = 1000ull * 1000ull * 1000ull * (usertime + systime)
+/ (unsigned long long)sysconf(_SC_CLK_TCK);
+if (lastCpu)
+*lastCpu = cpu;
+
+if (vm_rss)
+*vm_rss = rss * virGetSystemPageSizeKB();
+
+
+VIR_DEBUG("Got status for %d/%d user=%llu sys=%llu cpu=%d rss=%ld",
+  (int)pid, tid, usertime, systime, cpu, rss);
+
+VIR_FORCE_FCLOSE(pidinfo);
+
+return 0;
+}
+/*
+TODO: This method was cloned from qemuGetSchedInfo in src/qemu/qemu_driver.c.
+Need to refactor qemu driver to use this shared function.
+*/
+int virProcessGetSchedInfo(unsigned long long *cpuWait, pid_t pid, pid_t tid)
+{
+g_autofree char *proc = NULL;
+g_autofree char *data = NULL;
+char **lines = NULL;
+size_t i;
+int ret = -1;
+double val;
+
+*cpuWait = 0;
+
+/* In general, we cannot assume pid_t fits in int; but /proc parsing
+ * is specific to Linux where int works fine.  */
+if (tid)
+proc = g_strdup_printf("/proc/%d/task/%d/sched", (int)pid, (int)tid);
+else
+proc = g_strdup_printf("/proc/%d/sched", (int)pid);
+if (!proc)
+goto cleanup;
+ret = -1;
+
+/* The file is not guaranteed to exist (needs CONFIG_SCHED_DEBUG) */
+if (access(proc, R_OK) < 0) {
+ret = 0;
+goto cleanup;
+}
+
+if (virFileReadAll(proc, (1<<16), ) < 0)
+goto cleanup;
+
+lines = g_strsplit(data, "\n", 0);
+if (!lines)
+goto cleanup;
+
+for (i = 0; lines[i] != NULL; i++) {
+const char *line = lines[i];
+
+/* Needs CONFIG_SCHEDSTATS. The second check
+ * is the old name the kernel used in past */
+if (STRPREFIX(line, "se.statistics.wait_sum") ||
+STRPREFIX(line, "se.wait_sum")) {
+line = strchr(line, ':');
+if (!line) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Missing separator in sched info '%s'"),
+   lines[i]);
+goto cleanup;
+}
+line++;
+while (*line == ' ')
+line++;
+
+if (virStrToDouble(line, NULL, ) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Unable to parse sched info value '%s'"),
+   line);
+goto cleanup;
+}
+
+*cpuWait = (unsigned long long)(val * 100);
+break;
+}
+}
+
+ret = 0;
+
+ cleanup:
+g_strfreev(lines);
+return ret;
+}
diff --git a/src/util/virprocess.h b/src/util/virprocess.h
index 9910331a0c..3a7c4c2d64 100644
--- a/src/util/virprocess.h
+++ b/src/util/virprocess.h
@@ -127,3 +127,8 @@ typedef enum {
 } virProcessNamespaceFlags;
 
 int virProcessNamespaceAvailable(unsigned int ns);
+
+int virProcessGetStatInfo(unsigned 

[libvirt PATCH 06/13] ch_driver: domainGetVcpuPinInfo and nodeGetCPUMap

[Vineeth Pillai]

Add domainGetVcpuPinInfo and nodeGetCPUMap callbacks to ch driver

Signed-off-by: Vineeth Pillai 
Signed-off-by: Praveen K Paladugu 
---
 src/ch/ch_domain.h | 12 +--
 src/ch/ch_driver.c | 54 ++
 2 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h
index d9c9d34a19..e35777a9ec 100644
--- a/src/ch/ch_domain.h
+++ b/src/ch/ch_domain.h
@@ -23,6 +23,7 @@
 #include "ch_conf.h"
 #include "ch_monitor.h"
 #include "virchrdev.h"
+#include "vircgroup.h"
 
 /* Give up waiting for mutex after 30 seconds */
 #define CH_JOB_WAIT_TIME (1000ull * 30)
@@ -52,9 +53,16 @@ typedef struct _virCHDomainObjPrivate virCHDomainObjPrivate;
 struct _virCHDomainObjPrivate {
 struct virCHDomainJobObj job;
 
-virCHMonitor *monitor;
+virChrdevs *chrdevs;
+
+virCgroup *cgroup;
 
- virChrdevs *chrdevs;
+virCHDriver *driver;
+virCHMonitor *monitor;
+char *machineName;
+virBitmap *autoNodeset;
+virBitmap *autoCpuset;
+virChrdevs *devs;
 };
 
 #define CH_DOMAIN_PRIVATE(vm) \
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 8ea5ce393d..62ca6c1994 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -990,6 +990,58 @@ chDomainGetMaxVcpus(virDomainPtr dom)
 return chDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE | 
VIR_DOMAIN_VCPU_MAXIMUM));
 }
 
+static int
+chDomainGetVcpuPinInfo(virDomain *dom,
+   int ncpumaps,
+   unsigned char *cpumaps,
+   int maplen,
+   unsigned int flags)
+{
+virDomainObj *vm = NULL;
+virDomainDef *def;
+bool live;
+int ret = -1;
+g_autoptr(virBitmap) hostcpus = NULL;
+virBitmap *autoCpuset = NULL;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+if (!(vm = chDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainGetVcpuPinInfoEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (!(def = virDomainObjGetOneDefState(vm, flags, )))
+goto cleanup;
+
+if (!(hostcpus = virHostCPUGetAvailableCPUsBitmap()))
+goto cleanup;
+
+if (live)
+autoCpuset = CH_DOMAIN_PRIVATE(vm)->autoCpuset;
+
+ret = virDomainDefGetVcpuPinInfoHelper(def, maplen, ncpumaps, cpumaps,
+   hostcpus, autoCpuset);
+cleanup:
+virDomainObjEndAPI();
+return ret;
+}
+
+static int
+chNodeGetCPUMap(virConnectPtr conn,
+  unsigned char **cpumap,
+  unsigned int *online,
+  unsigned int flags)
+{
+if (virNodeGetCPUMapEnsureACL(conn) < 0)
+return -1;
+
+return virHostCPUGetMap(cpumap, online, flags);
+}
+
+
 static int
 chDomainHelperGetVcpus(virDomainObj *vm,
virVcpuInfoPtr info,
@@ -1126,6 +1178,8 @@ static virHypervisorDriver chHypervisorDriver = {
 .domainGetVcpus = chDomainGetVcpus, /* 7.9.0 */
 .domainGetVcpusFlags = chDomainGetVcpusFlags,   /* 7.9.0 */
 .domainGetMaxVcpus = chDomainGetMaxVcpus,   /* 7.9.0 */
+.domainGetVcpuPinInfo = chDomainGetVcpuPinInfo, /* 7.9.0 */
+.nodeGetCPUMap = chNodeGetCPUMap,   /* 7.9.0 */
 };
 
 static virConnectDriver chConnectDriver = {
-- 
2.27.0

[libvirt PATCH 12/13] ch_process: Setup emulator and iothread settings

[Vineeth Pillai]

From: Praveen K Paladugu 

using virCHProcessSetupPid

Signed-off-by: Praveen K Paladugu 
Signed-off-by: Vineeth Pillai 
---
 src/ch/ch_monitor.c | 60 ++
 src/ch/ch_monitor.h |  2 ++
 src/ch/ch_process.c | 78 +++--
 3 files changed, 138 insertions(+), 2 deletions(-)

diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index 095779cb3f..924d510a2f 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -920,3 +920,63 @@ virCHMonitorResumeVM(virCHMonitor *mon)
 {
 return virCHMonitorPutNoContent(mon, URL_VM_RESUME);
 }
+
+/**
+ * virCHMonitorGetIOThreads:
+ * @mon: Pointer to the monitor
+ * @iothreads: Location to return array of IOThreadInfo data
+ *
+ * Retrieve the list of iothreads defined/running for the machine
+ *
+ * Returns count of IOThreadInfo structures on success
+ *-1 on error.
+ */
+int virCHMonitorGetIOThreads(virCHMonitor *mon,
+virDomainIOThreadInfo ***iothreads)
+{
+size_t nthreads = 0, niothreads = 0;
+int thd_index;
+virDomainIOThreadInfo **iothreadinfolist = NULL, *iothreadinfo = NULL;
+
+*iothreads = NULL;
+nthreads = virCHMonitorRefreshThreadInfo(mon);
+
+iothreadinfolist = g_new0(virDomainIOThreadInfo*, nthreads);
+
+for (thd_index = 0; thd_index < nthreads; thd_index++) {
+virBitmap *map = NULL;
+if (mon->threads[thd_index].type == virCHThreadTypeIO) {
+iothreadinfo = g_new0(virDomainIOThreadInfo, 1);
+
+iothreadinfo->iothread_id = mon->threads[thd_index].ioInfo.tid;
+
+if (!(map = virProcessGetAffinity(iothreadinfo->iothread_id)))
+goto cleanup;
+
+if (virBitmapToData(map, &(iothreadinfo->cpumap),
+&(iothreadinfo->cpumaplen)) < 0) {
+virBitmapFree(map);
+goto cleanup;
+}
+virBitmapFree(map);
+//Append to iothreadinfolist
+iothreadinfolist[niothreads] = iothreadinfo;
+niothreads++;
+}
+}
+VIR_DELETE_ELEMENT_INPLACE(iothreadinfolist,
+   niothreads, nthreads);
+*iothreads = iothreadinfolist;
+VIR_DEBUG("niothreads = %ld", niothreads);
+return niothreads;
+
+cleanup:
+if (iothreadinfolist) {
+for (thd_index = 0; thd_index < niothreads; thd_index++)
+VIR_FREE(iothreadinfolist[thd_index]);
+VIR_FREE(iothreadinfolist);
+}
+if (iothreadinfo)
+VIR_FREE(iothreadinfo);
+return -1;
+}
diff --git a/src/ch/ch_monitor.h b/src/ch/ch_monitor.h
index f8c3fa75e8..98edb0faf9 100644
--- a/src/ch/ch_monitor.h
+++ b/src/ch/ch_monitor.h
@@ -118,3 +118,5 @@ int virCHMonitorGetCPUInfo(virCHMonitor *mon,
size_t maxvcpus);
 size_t virCHMonitorGetThreadInfo(virCHMonitor *mon, bool refresh,
  virCHMonitorThreadInfo **threads);
+int virCHMonitorGetIOThreads(virCHMonitor *mon,
+virDomainIOThreadInfo ***iothreads);
diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c
index 8dce737adb..25c2910f9c 100644
--- a/src/ch/ch_process.c
+++ b/src/ch/ch_process.c
@@ -41,7 +41,6 @@ VIR_LOG_INIT("ch.ch_process");
 #define START_VM_POSTFIX ": starting up vm\n"
 
 
-
 static virCHMonitor *
 virCHProcessConnectMonitor(virCHDriver *driver,
virDomainObj *vm)
@@ -131,7 +130,6 @@ virCHProcessUpdateInfo(virDomainObj *vm)
 virCHProcessUpdateConsole(vm, info);
 
 virJSONValueFree(info);
-
 return 0;
 }
 
@@ -313,6 +311,74 @@ virCHProcessSetupPid(virDomainObj *vm,
 return ret;
 }
 
+static int
+virCHProcessSetupIOThread(virDomainObj *vm,
+ virDomainIOThreadInfo *iothread)
+{
+virCHDomainObjPrivate *priv = vm->privateData;
+return virCHProcessSetupPid(vm, iothread->iothread_id,
+   VIR_CGROUP_THREAD_IOTHREAD,
+   iothread->iothread_id,
+   priv->autoCpuset, // This should be updated 
when CLH supports accepting
+ // iothread settings from input domain 
definition
+   vm->def->cputune.iothread_period,
+   vm->def->cputune.iothread_quota,
+   NULL); // CLH doesn't allow choosing a 
scheduler for iothreads.
+}
+
+static int
+virCHProcessSetupIOThreads(virDomainObj *vm)
+{
+virCHDomainObjPrivate *priv = vm->privateData;
+virDomainIOThreadInfo **iothreads = NULL;
+size_t i;
+size_t  niothreads;
+
+niothreads = virCHMonitorGetIOThreads(priv->monitor, );
+for (i = 0; i < niothreads; i++) {
+VIR_DEBUG("IOThread index = %ld , tid = %d", i, 
iothreads[i]->iothread_id);
+if (virCHProcessSetupIOThread(vm, iothreads[i]) < 0)
+return -1;
+}
+  

Re: [PATCH 3/9] qapi: Eliminate QCO_NO_OPTIONS for a slight simplification

[Juan Quintela]

Markus Armbruster  wrote:
> Signed-off-by: Markus Armbruster 

Reviewed-by: Juan Quintela 

[libvirt PATCH 05/13] ch_driver, ch_domain: vcpu info getter callbacks

[Vineeth Pillai]

Signed-off-by: Vineeth Pillai 
Signed-off-by: Praveen K Paladugu 
---
 src/ch/ch_domain.c |  25 
 src/ch/ch_domain.h |   4 ++
 src/ch/ch_driver.c | 138 +
 3 files changed, 167 insertions(+)

diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c
index fedde4581b..c0b0b1005a 100644
--- a/src/ch/ch_domain.c
+++ b/src/ch/ch_domain.c
@@ -338,3 +338,28 @@ virCHDomainGetMonitor(virDomainObj *vm)
 {
 return CH_DOMAIN_PRIVATE(vm)->monitor;
 }
+
+pid_t
+virCHDomainGetVcpuPid(virDomainObj *vm,
+ unsigned int vcpuid)
+{
+virDomainVcpuDef *vcpu = virDomainDefGetVcpu(vm->def, vcpuid);
+return CH_DOMAIN_VCPU_PRIVATE(vcpu)->tid;
+}
+
+bool
+virCHDomainHasVcpuPids(virDomainObj *vm)
+{
+size_t i;
+size_t maxvcpus = virDomainDefGetVcpusMax(vm->def);
+virDomainVcpuDef *vcpu;
+
+for (i = 0; i < maxvcpus; i++) {
+vcpu = virDomainDefGetVcpu(vm->def, i);
+
+if (CH_DOMAIN_VCPU_PRIVATE(vcpu)->tid > 0)
+return true;
+}
+
+return false;
+}
diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h
index 75b9933130..d9c9d34a19 100644
--- a/src/ch/ch_domain.h
+++ b/src/ch/ch_domain.h
@@ -82,3 +82,7 @@ virCHDomainObjBeginJob(virDomainObj *obj, enum virCHDomainJob 
job)
 
 void
 virCHDomainObjEndJob(virDomainObj *obj);
+
+int virCHDomainRefreshVcpuInfo(virDomainObj *vm);
+pid_t virCHDomainGetVcpuPid(virDomainObj *vm, unsigned int vcpuid);
+bool virCHDomainHasVcpuPids(virDomainObj *vm);
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 1824d2fd16..8ea5ce393d 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -952,6 +952,141 @@ static int chStateInitialize(bool privileged,
 return ret;
 }
 
+static int
+chDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags)
+{
+virDomainObj *vm;
+virDomainDef *def;
+int ret = -1;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_DOMAIN_AFFECT_CONFIG |
+  VIR_DOMAIN_VCPU_MAXIMUM |
+  VIR_DOMAIN_VCPU_GUEST, -1);
+
+if (!(vm = chDomObjFromDomain(dom)))
+return -1;
+
+if (virDomainGetVcpusFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
+goto cleanup;
+
+if (!(def = virDomainObjGetOneDef(vm, flags)))
+goto cleanup;
+
+if (flags & VIR_DOMAIN_VCPU_MAXIMUM)
+ret = virDomainDefGetVcpusMax(def);
+else
+ret = virDomainDefGetVcpus(def);
+
+
+cleanup:
+virDomainObjEndAPI();
+return ret;
+}
+
+static int
+chDomainGetMaxVcpus(virDomainPtr dom)
+{
+return chDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE | 
VIR_DOMAIN_VCPU_MAXIMUM));
+}
+
+static int
+chDomainHelperGetVcpus(virDomainObj *vm,
+   virVcpuInfoPtr info,
+   unsigned long long *cpuwait,
+   int maxinfo,
+   unsigned char *cpumaps,
+   int maplen)
+{
+size_t ncpuinfo = 0;
+size_t i;
+
+if (maxinfo == 0)
+return 0;
+
+if (!virCHDomainHasVcpuPids(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   "%s", _("cpu affinity is not supported"));
+return -1;
+}
+
+if (info)
+memset(info, 0, sizeof(*info) * maxinfo);
+
+if (cpumaps)
+memset(cpumaps, 0, sizeof(*cpumaps) * maxinfo);
+
+for (i = 0; i < virDomainDefGetVcpusMax(vm->def) && ncpuinfo < maxinfo; 
i++) {
+virDomainVcpuDef *vcpu = virDomainDefGetVcpu(vm->def, i);
+pid_t vcpupid = virCHDomainGetVcpuPid(vm, i);
+virVcpuInfoPtr vcpuinfo = info + ncpuinfo;
+
+if (!vcpu->online)
+continue;
+
+if (info) {
+vcpuinfo->number = i;
+vcpuinfo->state = VIR_VCPU_RUNNING;
+if (virProcessGetStatInfo(>cpuTime,
+  >cpu, NULL,
+  vm->pid, vcpupid) < 0) {
+virReportSystemError(errno, "%s",
+ _("cannot get vCPU placement & pCPU 
time"));
+return -1;
+}
+}
+
+if (cpumaps) {
+unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, ncpuinfo);
+virBitmap *map = NULL;
+
+if (!(map = virProcessGetAffinity(vcpupid)))
+return -1;
+
+virBitmapToDataBuf(map, cpumap, maplen);
+virBitmapFree(map);
+}
+
+if (cpuwait) {
+if (virProcessGetSchedInfo(&(cpuwait[ncpuinfo]), vm->pid, vcpupid) 
< 0)
+return -1;
+}
+
+ncpuinfo++;
+}
+
+return ncpuinfo;
+}
+
+static int
+chDomainGetVcpus(virDomainPtr dom,
+   virVcpuInfoPtr info,
+   int maxinfo,
+   unsigned char *cpumaps,
+   int maplen)
+{
+virDomainObj *vm;
+int ret = -1;
+
+if (!(vm = chDomObjFromDomain(dom)))
+goto cleanup;
+
+if 

[libvirt PATCH 13/13] ch_driver: emulator threadinfo & pinning callbacks

[Vineeth Pillai]

From: Praveen K Paladugu 

Signed-off-by: Vineeth Pillai 
---
 src/ch/ch_driver.c | 154 +
 1 file changed, 154 insertions(+)

diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 97dfda85e1..786e2292a5 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -1321,6 +1321,158 @@ chDomainPinVcpu(virDomainPtr dom,
   VIR_DOMAIN_AFFECT_LIVE);
 }
 
+
+
+static int
+chDomainGetEmulatorPinInfo(virDomainPtr dom,
+ unsigned char *cpumaps,
+ int maplen,
+ unsigned int flags)
+{
+virDomainObj *vm = NULL;
+virDomainDef *def;
+virCHDomainObjPrivate *priv;
+bool live;
+int ret = -1;
+virBitmap *cpumask = NULL;
+g_autoptr(virBitmap) bitmap = NULL;
+virBitmap *autoCpuset = NULL;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+if (!(vm = chDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainGetEmulatorPinInfoEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (!(def = virDomainObjGetOneDefState(vm, flags, )))
+goto cleanup;
+
+if (live) {
+priv = vm->privateData;
+autoCpuset = priv->autoCpuset;
+}
+if (def->cputune.emulatorpin) {
+cpumask = def->cputune.emulatorpin;
+} else if (def->cpumask) {
+cpumask = def->cpumask;
+} else if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO &&
+   autoCpuset) {
+cpumask = autoCpuset;
+} else {
+if (!(bitmap = virHostCPUGetAvailableCPUsBitmap()))
+goto cleanup;
+cpumask = bitmap;
+}
+
+virBitmapToDataBuf(cpumask, cpumaps, maplen);
+
+ret = 1;
+
+ cleanup:
+virDomainObjEndAPI();
+return ret;
+}
+
+static int
+chDomainPinEmulator(virDomainPtr dom,
+  unsigned char *cpumap,
+  int maplen,
+  unsigned int flags)
+{
+virCHDriver *driver = dom->conn->privateData;
+virDomainObj *vm;
+virCgroup *cgroup_emulator = NULL;
+virDomainDef *def;
+virDomainDef *persistentDef;
+int ret = -1;
+virCHDomainObjPrivate *priv;
+virBitmap *pcpumap = NULL;
+g_autoptr(virCHDriverConfig) cfg = NULL;
+g_autofree char *str = NULL;
+virTypedParameterPtr eventParams = NULL;
+int eventNparams = 0;
+int eventMaxparams = 0;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+cfg = virCHDriverGetConfig(driver);
+
+if (!(vm = chDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainPinEmulatorEnsureACL(dom->conn, vm->def, flags) < 0)
+goto cleanup;
+
+if (virCHDomainObjBeginJob(vm, CH_JOB_MODIFY) < 0)
+goto cleanup;
+
+if (virDomainObjGetDefs(vm, flags, , ) < 0)
+goto endjob;
+
+priv = vm->privateData;
+
+if (!(pcpumap = virBitmapNewData(cpumap, maplen)))
+goto endjob;
+
+if (virBitmapIsAllClear(pcpumap)) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("Empty cpu list for pinning"));
+goto endjob;
+}
+
+if (def) {
+if (virCgroupHasController(priv->cgroup, 
VIR_CGROUP_CONTROLLER_CPUSET)) {
+if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR,
+   0, false, _emulator) < 0)
+goto endjob;
+
+if (chSetupCgroupCpusetCpus(cgroup_emulator, pcpumap) < 0) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("failed to set cpuset.cpus in cgroup"
+ " for emulator threads"));
+goto endjob;
+}
+}
+
+if (virProcessSetAffinity(vm->pid, pcpumap, false) < 0)
+goto endjob;
+
+virBitmapFree(def->cputune.emulatorpin);
+def->cputune.emulatorpin = NULL;
+
+if (!(def->cputune.emulatorpin = virBitmapNewCopy(pcpumap)))
+goto endjob;
+
+if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0)
+goto endjob;
+
+str = virBitmapFormat(pcpumap);
+if (virTypedParamsAddString(, ,
+,
+VIR_DOMAIN_TUNABLE_CPU_EMULATORPIN,
+str) < 0)
+goto endjob;
+
+}
+
+
+ret = 0;
+
+endjob:
+virCHDomainObjEndJob(vm);
+
+cleanup:
+if (cgroup_emulator)
+virCgroupFree(cgroup_emulator);
+virBitmapFree(pcpumap);
+virDomainObjEndAPI();
+return ret;
+}
+
 #define CH_NB_NUMA_PARAM 2
 
 static int
@@ -1639,6 +1791,8 @@ static virHypervisorDriver chHypervisorDriver = {
 .domainGetVcpuPinInfo = chDomainGetVcpuPinInfo, /* 7.9.0 */
 .domainPinVcpu = chDomainPinVcpu,   

[libvirt PATCH 00/13] cgroup and thread management in ch driver.

[Vineeth Pillai]

This patchset adds support for cgroup management of ch threads. This version
correctly manages cgroups for vcpu and emulator threads created by ch. cgroup
management for iothreads is not yet supported.

Along with cgroup management, this patchset also enables support for pinning
vcpu and emulator threads to selected host cpus.

Praveen K Paladugu (2):
  ch_process: Setup emulator and iothread settings
  ch_driver: emulator threadinfo & pinning callbacks

Vineeth Pillai (11):
  util: Helper functions to get process info
  ch: Explicitly link to virt_util_lib
  ch_domain: add virCHDomainGetMonitor helper method
  ch_domain: add methods to manage private vcpu data
  ch_driver,ch_domain: vcpu info getter callbacks
  ch_driver: domainGetVcpuPinInfo and nodeGetCPUMap
  ch_monitor: Get nicindexes in prep for cgroup mgmt
  ch_cgroup: methods for cgroup mgmt in ch driver
  ch_driver,ch_domain: vcpupin callback in ch driver
  ch_driver: enable typed param string for numatune
  ch_driver: add numatune callbacks for CH driver

 po/POTFILES.in|   1 +
 src/ch/ch_cgroup.c| 457 
 src/ch/ch_cgroup.h|  45 +++
 src/ch/ch_conf.c  |   2 +
 src/ch/ch_conf.h  |   9 +-
 src/ch/ch_domain.c| 170 -
 src/ch/ch_domain.h|  32 +-
 src/ch/ch_driver.c| 810 +-
 src/ch/ch_monitor.c   | 254 -
 src/ch/ch_monitor.h   |  60 +++-
 src/ch/ch_process.c   | 368 ++-
 src/ch/ch_process.h   |   3 +
 src/ch/meson.build|   6 +
 src/util/virprocess.c | 136 +++
 src/util/virprocess.h |   5 +
 15 files changed, 2329 insertions(+), 29 deletions(-)
 create mode 100644 src/ch/ch_cgroup.c
 create mode 100644 src/ch/ch_cgroup.h

-- 
2.27.0

[libvirt PATCH 09/13] ch_driver, ch_domain: vcpupin callback in ch driver

[Vineeth Pillai]

Signed-off-by: Vineeth Pillai 
Signed-off-by: Praveen K Paladugu 
---
 src/ch/ch_domain.c |  30 +
 src/ch/ch_domain.h |   1 +
 src/ch/ch_driver.c | 151 +
 3 files changed, 182 insertions(+)

diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c
index d0aaeed1f4..9ad00583aa 100644
--- a/src/ch/ch_domain.c
+++ b/src/ch/ch_domain.c
@@ -20,6 +20,7 @@
 
 #include 
 
+#include "datatypes.h"
 #include "ch_domain.h"
 #include "domain_driver.h"
 #include "viralloc.h"
@@ -420,3 +421,32 @@ char *virCHDomainGetMachineName(virDomainObj *vm)
 
 return ret;
 }
+
+/**
+ * virCHDomainObjFromDomain:
+ * @domain: Domain pointer that has to be looked up
+ *
+ * This function looks up @domain and returns the appropriate virDomainObjPtr
+ * that has to be released by calling virDomainObjEndAPI().
+ *
+ * Returns the domain object with incremented reference counter which is locked
+ * on success, NULL otherwise.
+ */
+virDomainObj *
+virCHDomainObjFromDomain(virDomainPtr domain)
+{
+virDomainObj *vm;
+virCHDriver *driver = domain->conn->privateData;
+char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+vm = virDomainObjListFindByUUID(driver->domains, domain->uuid);
+if (!vm) {
+virUUIDFormat(domain->uuid, uuidstr);
+virReportError(VIR_ERR_NO_DOMAIN,
+   _("no domain with matching uuid '%s' (%s)"),
+   uuidstr, domain->name);
+return NULL;
+}
+
+return vm;
+}
diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h
index 2ce3e2cef3..db1451405b 100644
--- a/src/ch/ch_domain.h
+++ b/src/ch/ch_domain.h
@@ -95,3 +95,4 @@ pid_t virCHDomainGetVcpuPid(virDomainObj *vm, unsigned int 
vcpuid);
 bool virCHDomainHasVcpuPids(virDomainObj *vm);
 
 char *virCHDomainGetMachineName(virDomainObj *vm);
+virDomainObj *virCHDomainObjFromDomain(virDomain *domain);
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index ca854da123..7f3cf6dbef 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -25,6 +25,7 @@
 #include "ch_driver.h"
 #include "ch_monitor.h"
 #include "ch_process.h"
+#include "ch_cgroup.h"
 #include "datatypes.h"
 #include "driver.h"
 #include "viraccessapicheck.h"
@@ -1141,6 +1142,154 @@ chDomainGetVcpus(virDomainPtr dom,
 return ret;
 }
 
+static int
+chDomainPinVcpuLive(virDomainObj *vm,
+virDomainDef *def,
+int vcpu,
+virCHDriver *driver,
+virCHDriverConfig *cfg,
+virBitmap *cpumap)
+{
+virBitmap *tmpmap = NULL;
+virDomainVcpuDef *vcpuinfo;
+virCHDomainObjPrivate *priv = vm->privateData;
+virCgroup *cgroup_vcpu = NULL;
+g_autofree char *str = NULL;
+int ret = -1;
+
+if (!virCHDomainHasVcpuPids(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   "%s", _("cpu affinity is not supported"));
+goto cleanup;
+}
+
+if (!(vcpuinfo = virDomainDefGetVcpu(def, vcpu))) {
+virReportError(VIR_ERR_INVALID_ARG,
+   _("vcpu %d is out of range of live cpu count %d"),
+   vcpu, virDomainDefGetVcpusMax(def));
+goto cleanup;
+}
+
+if (!(tmpmap = virBitmapNewCopy(cpumap)))
+goto cleanup;
+
+if (!(str = virBitmapFormat(cpumap)))
+goto cleanup;
+
+if (vcpuinfo->online) {
+/* Configure the corresponding cpuset cgroup before set affinity. */
+if (virCgroupHasController(priv->cgroup, 
VIR_CGROUP_CONTROLLER_CPUSET)) {
+if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, vcpu,
+   false, _vcpu) < 0)
+goto cleanup;
+if (chSetupCgroupCpusetCpus(cgroup_vcpu, cpumap) < 0)
+goto cleanup;
+}
+
+if (virProcessSetAffinity(virCHDomainGetVcpuPid(vm, vcpu), cpumap, 
false) < 0)
+goto cleanup;
+}
+
+virBitmapFree(vcpuinfo->cpumask);
+vcpuinfo->cpumask = tmpmap;
+tmpmap = NULL;
+
+if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0)
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+virBitmapFree(tmpmap);
+virCgroupFree(cgroup_vcpu);
+return ret;
+}
+
+
+static int
+chDomainPinVcpuFlags(virDomainPtr dom,
+ unsigned int vcpu,
+ unsigned char *cpumap,
+ int maplen,
+ unsigned int flags)
+{
+virCHDriver *driver = dom->conn->privateData;
+virDomainObj *vm;
+virDomainDef *def;
+virDomainDef *persistentDef;
+int ret = -1;
+virBitmap *pcpumap = NULL;
+virDomainVcpuDef *vcpuinfo = NULL;
+g_autoptr(virCHDriverConfig) cfg = NULL;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+cfg = virCHDriverGetConfig(driver);
+
+if (!(vm = virCHDomainObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainPinVcpuFlagsEnsureACL(dom->conn, 

[libvirt PATCH 10/13] ch_driver: enable typed param string for numatune

[Vineeth Pillai]

Enable support of VIR_DRV_FEATURE_TYPED_PARAM_STRING to enable numatune

Signed-off-by: Vineeth Pillai 
Signed-off-by: Praveen K Paladugu 
---
 src/ch/ch_driver.c | 31 +++
 1 file changed, 31 insertions(+)

diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 7f3cf6dbef..9ad23c1710 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -955,6 +955,36 @@ static int chStateInitialize(bool privileged,
 return ret;
 }
 
+/* Which features are supported by this driver? */
+static int
+chConnectSupportsFeature(virConnectPtr conn, int feature)
+{
+if (virConnectSupportsFeatureEnsureACL(conn) < 0)
+return -1;
+
+switch ((virDrvFeature) feature) {
+case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
+return 1;
+case VIR_DRV_FEATURE_MIGRATION_V2:
+case VIR_DRV_FEATURE_MIGRATION_V3:
+case VIR_DRV_FEATURE_MIGRATION_P2P:
+case VIR_DRV_FEATURE_MIGRATE_CHANGE_PROTECTION:
+case VIR_DRV_FEATURE_FD_PASSING:
+case VIR_DRV_FEATURE_XML_MIGRATABLE:
+case VIR_DRV_FEATURE_MIGRATION_OFFLINE:
+case VIR_DRV_FEATURE_MIGRATION_PARAMS:
+case VIR_DRV_FEATURE_MIGRATION_DIRECT:
+case VIR_DRV_FEATURE_MIGRATION_V1:
+case VIR_DRV_FEATURE_PROGRAM_KEEPALIVE:
+case VIR_DRV_FEATURE_REMOTE:
+case VIR_DRV_FEATURE_REMOTE_CLOSE_CALLBACK:
+case VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK:
+case VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER:
+default:
+return 0;
+}
+}
+
 static int
 chDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags)
 {
@@ -1303,6 +1333,7 @@ static virHypervisorDriver chHypervisorDriver = {
 .connectListAllDomains = chConnectListAllDomains,   /* 7.5.0 */
 .connectListDomains = chConnectListDomains, /* 7.5.0 */
 .connectGetCapabilities = chConnectGetCapabilities, /* 7.5.0 */
+.connectSupportsFeature = chConnectSupportsFeature, /* 7.8.0 */
 .domainCreateXML = chDomainCreateXML,   /* 7.5.0 */
 .domainCreate = chDomainCreate, /* 7.5.0 */
 .domainCreateWithFlags = chDomainCreateWithFlags,   /* 7.5.0 */
-- 
2.27.0

[libvirt PATCH 13/13] ch_driver: emulator threadinfo & pinning callbacks

[Vineeth Pillai]

From: Praveen K Paladugu 

Signed-off-by: Vineeth Pillai 
---
 src/ch/ch_driver.c | 154 +
 1 file changed, 154 insertions(+)

diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 97dfda85e1..786e2292a5 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -1321,6 +1321,158 @@ chDomainPinVcpu(virDomainPtr dom,
   VIR_DOMAIN_AFFECT_LIVE);
 }
 
+
+
+static int
+chDomainGetEmulatorPinInfo(virDomainPtr dom,
+ unsigned char *cpumaps,
+ int maplen,
+ unsigned int flags)
+{
+virDomainObj *vm = NULL;
+virDomainDef *def;
+virCHDomainObjPrivate *priv;
+bool live;
+int ret = -1;
+virBitmap *cpumask = NULL;
+g_autoptr(virBitmap) bitmap = NULL;
+virBitmap *autoCpuset = NULL;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+if (!(vm = chDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainGetEmulatorPinInfoEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (!(def = virDomainObjGetOneDefState(vm, flags, )))
+goto cleanup;
+
+if (live) {
+priv = vm->privateData;
+autoCpuset = priv->autoCpuset;
+}
+if (def->cputune.emulatorpin) {
+cpumask = def->cputune.emulatorpin;
+} else if (def->cpumask) {
+cpumask = def->cpumask;
+} else if (vm->def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO &&
+   autoCpuset) {
+cpumask = autoCpuset;
+} else {
+if (!(bitmap = virHostCPUGetAvailableCPUsBitmap()))
+goto cleanup;
+cpumask = bitmap;
+}
+
+virBitmapToDataBuf(cpumask, cpumaps, maplen);
+
+ret = 1;
+
+ cleanup:
+virDomainObjEndAPI();
+return ret;
+}
+
+static int
+chDomainPinEmulator(virDomainPtr dom,
+  unsigned char *cpumap,
+  int maplen,
+  unsigned int flags)
+{
+virCHDriver *driver = dom->conn->privateData;
+virDomainObj *vm;
+virCgroup *cgroup_emulator = NULL;
+virDomainDef *def;
+virDomainDef *persistentDef;
+int ret = -1;
+virCHDomainObjPrivate *priv;
+virBitmap *pcpumap = NULL;
+g_autoptr(virCHDriverConfig) cfg = NULL;
+g_autofree char *str = NULL;
+virTypedParameterPtr eventParams = NULL;
+int eventNparams = 0;
+int eventMaxparams = 0;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+cfg = virCHDriverGetConfig(driver);
+
+if (!(vm = chDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainPinEmulatorEnsureACL(dom->conn, vm->def, flags) < 0)
+goto cleanup;
+
+if (virCHDomainObjBeginJob(vm, CH_JOB_MODIFY) < 0)
+goto cleanup;
+
+if (virDomainObjGetDefs(vm, flags, , ) < 0)
+goto endjob;
+
+priv = vm->privateData;
+
+if (!(pcpumap = virBitmapNewData(cpumap, maplen)))
+goto endjob;
+
+if (virBitmapIsAllClear(pcpumap)) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+   _("Empty cpu list for pinning"));
+goto endjob;
+}
+
+if (def) {
+if (virCgroupHasController(priv->cgroup, 
VIR_CGROUP_CONTROLLER_CPUSET)) {
+if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR,
+   0, false, _emulator) < 0)
+goto endjob;
+
+if (chSetupCgroupCpusetCpus(cgroup_emulator, pcpumap) < 0) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("failed to set cpuset.cpus in cgroup"
+ " for emulator threads"));
+goto endjob;
+}
+}
+
+if (virProcessSetAffinity(vm->pid, pcpumap, false) < 0)
+goto endjob;
+
+virBitmapFree(def->cputune.emulatorpin);
+def->cputune.emulatorpin = NULL;
+
+if (!(def->cputune.emulatorpin = virBitmapNewCopy(pcpumap)))
+goto endjob;
+
+if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0)
+goto endjob;
+
+str = virBitmapFormat(pcpumap);
+if (virTypedParamsAddString(, ,
+,
+VIR_DOMAIN_TUNABLE_CPU_EMULATORPIN,
+str) < 0)
+goto endjob;
+
+}
+
+
+ret = 0;
+
+endjob:
+virCHDomainObjEndJob(vm);
+
+cleanup:
+if (cgroup_emulator)
+virCgroupFree(cgroup_emulator);
+virBitmapFree(pcpumap);
+virDomainObjEndAPI();
+return ret;
+}
+
 #define CH_NB_NUMA_PARAM 2
 
 static int
@@ -1639,6 +1791,8 @@ static virHypervisorDriver chHypervisorDriver = {
 .domainGetVcpuPinInfo = chDomainGetVcpuPinInfo, /* 7.9.0 */
 .domainPinVcpu = chDomainPinVcpu,   

[libvirt PATCH 07/13] ch_monitor: Get nicindexes in prep for cgroup mgmt

[Vineeth Pillai]

Signed-off-by: Vineeth Pillai 
Signed-off-by: Praveen Paladugu 
---
 src/ch/ch_conf.h|  5 +++
 src/ch/ch_domain.c  | 26 +-
 src/ch/ch_domain.h  |  4 +--
 src/ch/ch_driver.c  |  4 ++-
 src/ch/ch_monitor.c | 85 +
 src/ch/ch_monitor.h | 14 +++-
 src/ch/ch_process.c |  6 +++-
 src/ch/meson.build  |  1 +
 8 files changed, 132 insertions(+), 13 deletions(-)

diff --git a/src/ch/ch_conf.h b/src/ch/ch_conf.h
index 37c36d9a09..49f286f97a 100644
--- a/src/ch/ch_conf.h
+++ b/src/ch/ch_conf.h
@@ -44,6 +44,11 @@ struct _virCHDriver
 {
 virMutex lock;
 
+bool privileged;
+
+/* Embedded Mode: Not yet supported */
+char *embeddedRoot;
+
 /* Require lock to get a reference on the object,
  * lockless access thereafter */
 virCaps *caps;
diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c
index c0b0b1005a..e1030800aa 100644
--- a/src/ch/ch_domain.c
+++ b/src/ch/ch_domain.c
@@ -21,10 +21,12 @@
 #include 
 
 #include "ch_domain.h"
+#include "domain_driver.h"
 #include "viralloc.h"
 #include "virchrdev.h"
 #include "virlog.h"
 #include "virtime.h"
+#include "virsystemd.h"
 
 #define VIR_FROM_THIS VIR_FROM_CH
 
@@ -136,7 +138,7 @@ virCHDomainObjEndJob(virDomainObj *obj)
 }
 
 static void *
-virCHDomainObjPrivateAlloc(void *opaque G_GNUC_UNUSED)
+virCHDomainObjPrivateAlloc(void *opaque)
 {
 virCHDomainObjPrivate *priv;
 
@@ -152,6 +154,7 @@ virCHDomainObjPrivateAlloc(void *opaque G_GNUC_UNUSED)
 g_free(priv);
 return NULL;
 }
+priv->driver = opaque;
 
 return priv;
 }
@@ -363,3 +366,24 @@ virCHDomainHasVcpuPids(virDomainObj *vm)
 
 return false;
 }
+
+char *virCHDomainGetMachineName(virDomainObj *vm)
+{
+virCHDomainObjPrivate *priv = CH_DOMAIN_PRIVATE(vm);
+virCHDriver *driver = priv->driver;
+char *ret = NULL;
+
+if (vm->pid > 0) {
+ret = virSystemdGetMachineNameByPID(vm->pid);
+if (!ret)
+virResetLastError();
+}
+
+if (!ret)
+ret = virDomainDriverGenerateMachineName("ch",
+ driver->embeddedRoot,
+ vm->def->id, vm->def->name,
+ driver->privileged);
+
+return ret;
+}
diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h
index e35777a9ec..3ac3421015 100644
--- a/src/ch/ch_domain.h
+++ b/src/ch/ch_domain.h
@@ -54,9 +54,7 @@ struct _virCHDomainObjPrivate {
 struct virCHDomainJobObj job;
 
 virChrdevs *chrdevs;
-
 virCgroup *cgroup;
-
 virCHDriver *driver;
 virCHMonitor *monitor;
 char *machineName;
@@ -94,3 +92,5 @@ virCHDomainObjEndJob(virDomainObj *obj);
 int virCHDomainRefreshVcpuInfo(virDomainObj *vm);
 pid_t virCHDomainGetVcpuPid(virDomainObj *vm, unsigned int vcpuid);
 bool virCHDomainHasVcpuPids(virDomainObj *vm);
+
+char *virCHDomainGetMachineName(virDomainObj *vm);
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 62ca6c1994..ca854da123 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -944,7 +944,9 @@ static int chStateInitialize(bool privileged,
 goto cleanup;
 }
 
-ret = VIR_DRV_STATE_INIT_COMPLETE;
+ch_driver->privileged = privileged;
+
+return VIR_DRV_STATE_INIT_COMPLETE;
 
  cleanup:
 if (ret != VIR_DRV_STATE_INIT_COMPLETE)
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index 691d1ce64b..c0ae031200 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -228,7 +228,8 @@ virCHMonitorBuildDisksJson(virJSONValue *content, 
virDomainDef *vmdef)
 }
 
 static int
-virCHMonitorBuildNetJson(virJSONValue *nets, virDomainNetDef *netdef)
+virCHMonitorBuildNetJson(virJSONValue *nets, virDomainNetDef *netdef,
+size_t *nnicindexes, int **nicindexes)
 {
 virDomainNetType netType = virDomainNetGetActualType(netdef);
 char macaddr[VIR_MAC_STRING_BUFLEN];
@@ -263,6 +264,18 @@ virCHMonitorBuildNetJson(virJSONValue *nets, 
virDomainNetDef *netdef)
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("ethernet type supports a single guest ip"));
 }
+
+/* network and bridge use a tap device, and direct uses a
+ * macvtap device
+ */
+if (nicindexes && nnicindexes && netdef->ifname) {
+int nicindex = 0;
+if (virNetDevGetIndex(netdef->ifname, ) < 0)
+return -1;
+else
+VIR_APPEND_ELEMENT(*nicindexes, *nnicindexes, nicindex);
+}
+
 break;
 case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
 if ((virDomainChrType)netdef->data.vhostuser->type != 
VIR_DOMAIN_CHR_TYPE_UNIX) {
@@ -331,7 +344,8 @@ virCHMonitorBuildNetJson(virJSONValue *nets, 
virDomainNetDef *netdef)
 }
 
 static int
-virCHMonitorBuildNetsJson(virJSONValue *content, virDomainDef *vmdef)
+virCHMonitorBuildNetsJson(virJSONValue *content, virDomainDef *vmdef,
+   

[libvirt PATCH 12/13] ch_process: Setup emulator and iothread settings

[Vineeth Pillai]

From: Praveen K Paladugu 

using virCHProcessSetupPid

Signed-off-by: Praveen K Paladugu 
Signed-off-by: Vineeth Pillai 
---
 src/ch/ch_monitor.c | 60 ++
 src/ch/ch_monitor.h |  2 ++
 src/ch/ch_process.c | 78 +++--
 3 files changed, 138 insertions(+), 2 deletions(-)

diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index 095779cb3f..924d510a2f 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -920,3 +920,63 @@ virCHMonitorResumeVM(virCHMonitor *mon)
 {
 return virCHMonitorPutNoContent(mon, URL_VM_RESUME);
 }
+
+/**
+ * virCHMonitorGetIOThreads:
+ * @mon: Pointer to the monitor
+ * @iothreads: Location to return array of IOThreadInfo data
+ *
+ * Retrieve the list of iothreads defined/running for the machine
+ *
+ * Returns count of IOThreadInfo structures on success
+ *-1 on error.
+ */
+int virCHMonitorGetIOThreads(virCHMonitor *mon,
+virDomainIOThreadInfo ***iothreads)
+{
+size_t nthreads = 0, niothreads = 0;
+int thd_index;
+virDomainIOThreadInfo **iothreadinfolist = NULL, *iothreadinfo = NULL;
+
+*iothreads = NULL;
+nthreads = virCHMonitorRefreshThreadInfo(mon);
+
+iothreadinfolist = g_new0(virDomainIOThreadInfo*, nthreads);
+
+for (thd_index = 0; thd_index < nthreads; thd_index++) {
+virBitmap *map = NULL;
+if (mon->threads[thd_index].type == virCHThreadTypeIO) {
+iothreadinfo = g_new0(virDomainIOThreadInfo, 1);
+
+iothreadinfo->iothread_id = mon->threads[thd_index].ioInfo.tid;
+
+if (!(map = virProcessGetAffinity(iothreadinfo->iothread_id)))
+goto cleanup;
+
+if (virBitmapToData(map, &(iothreadinfo->cpumap),
+&(iothreadinfo->cpumaplen)) < 0) {
+virBitmapFree(map);
+goto cleanup;
+}
+virBitmapFree(map);
+//Append to iothreadinfolist
+iothreadinfolist[niothreads] = iothreadinfo;
+niothreads++;
+}
+}
+VIR_DELETE_ELEMENT_INPLACE(iothreadinfolist,
+   niothreads, nthreads);
+*iothreads = iothreadinfolist;
+VIR_DEBUG("niothreads = %ld", niothreads);
+return niothreads;
+
+cleanup:
+if (iothreadinfolist) {
+for (thd_index = 0; thd_index < niothreads; thd_index++)
+VIR_FREE(iothreadinfolist[thd_index]);
+VIR_FREE(iothreadinfolist);
+}
+if (iothreadinfo)
+VIR_FREE(iothreadinfo);
+return -1;
+}
diff --git a/src/ch/ch_monitor.h b/src/ch/ch_monitor.h
index f8c3fa75e8..98edb0faf9 100644
--- a/src/ch/ch_monitor.h
+++ b/src/ch/ch_monitor.h
@@ -118,3 +118,5 @@ int virCHMonitorGetCPUInfo(virCHMonitor *mon,
size_t maxvcpus);
 size_t virCHMonitorGetThreadInfo(virCHMonitor *mon, bool refresh,
  virCHMonitorThreadInfo **threads);
+int virCHMonitorGetIOThreads(virCHMonitor *mon,
+virDomainIOThreadInfo ***iothreads);
diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c
index 8dce737adb..25c2910f9c 100644
--- a/src/ch/ch_process.c
+++ b/src/ch/ch_process.c
@@ -41,7 +41,6 @@ VIR_LOG_INIT("ch.ch_process");
 #define START_VM_POSTFIX ": starting up vm\n"
 
 
-
 static virCHMonitor *
 virCHProcessConnectMonitor(virCHDriver *driver,
virDomainObj *vm)
@@ -131,7 +130,6 @@ virCHProcessUpdateInfo(virDomainObj *vm)
 virCHProcessUpdateConsole(vm, info);
 
 virJSONValueFree(info);
-
 return 0;
 }
 
@@ -313,6 +311,74 @@ virCHProcessSetupPid(virDomainObj *vm,
 return ret;
 }
 
+static int
+virCHProcessSetupIOThread(virDomainObj *vm,
+ virDomainIOThreadInfo *iothread)
+{
+virCHDomainObjPrivate *priv = vm->privateData;
+return virCHProcessSetupPid(vm, iothread->iothread_id,
+   VIR_CGROUP_THREAD_IOTHREAD,
+   iothread->iothread_id,
+   priv->autoCpuset, // This should be updated 
when CLH supports accepting
+ // iothread settings from input domain 
definition
+   vm->def->cputune.iothread_period,
+   vm->def->cputune.iothread_quota,
+   NULL); // CLH doesn't allow choosing a 
scheduler for iothreads.
+}
+
+static int
+virCHProcessSetupIOThreads(virDomainObj *vm)
+{
+virCHDomainObjPrivate *priv = vm->privateData;
+virDomainIOThreadInfo **iothreads = NULL;
+size_t i;
+size_t  niothreads;
+
+niothreads = virCHMonitorGetIOThreads(priv->monitor, );
+for (i = 0; i < niothreads; i++) {
+VIR_DEBUG("IOThread index = %ld , tid = %d", i, 
iothreads[i]->iothread_id);
+if (virCHProcessSetupIOThread(vm, iothreads[i]) < 0)
+return -1;
+}
+  

[libvirt PATCH 04/13] ch_domain: add methods to manage private vcpu data

[Vineeth Pillai]

Signed-off-by: Vineeth Pillai 
Signed-off-by: Praveen K Paladugu 
---
 src/ch/ch_domain.c | 50 +-
 src/ch/ch_domain.h | 11 ++
 2 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c
index ae79b47253..fedde4581b 100644
--- a/src/ch/ch_domain.c
+++ b/src/ch/ch_domain.c
@@ -166,11 +166,6 @@ virCHDomainObjPrivateFree(void *data)
 g_free(priv);
 }
 
-virDomainXMLPrivateDataCallbacks virCHDriverPrivateDataCallbacks = {
-.alloc = virCHDomainObjPrivateAlloc,
-.free = virCHDomainObjPrivateFree,
-};
-
 static int
 virCHDomainDefPostParseBasic(virDomainDef *def,
  void *opaque G_GNUC_UNUSED)
@@ -187,6 +182,45 @@ virCHDomainDefPostParseBasic(virDomainDef *def,
 return 0;
 }
 
+static virClass *virCHDomainVcpuPrivateClass;
+static void virCHDomainVcpuPrivateDispose(void *obj);
+
+static int
+virCHDomainVcpuPrivateOnceInit(void)
+{
+if (!VIR_CLASS_NEW(virCHDomainVcpuPrivate, virClassForObject()))
+return -1;
+
+return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(virCHDomainVcpuPrivate);
+
+static virObject *
+virCHDomainVcpuPrivateNew(void)
+{
+virCHDomainVcpuPrivate *priv;
+
+if (virCHDomainVcpuPrivateInitialize() < 0)
+return NULL;
+
+if (!(priv = virObjectNew(virCHDomainVcpuPrivateClass)))
+return NULL;
+
+return (virObject *) priv;
+}
+
+
+static void
+virCHDomainVcpuPrivateDispose(void *obj)
+{
+virCHDomainVcpuPrivate *priv = obj;
+
+priv->tid = 0;
+
+return;
+}
+
 static int
 virCHDomainDefPostParse(virDomainDef *def,
 unsigned int parseFlags G_GNUC_UNUSED,
@@ -205,6 +239,12 @@ virCHDomainDefPostParse(virDomainDef *def,
 return 0;
 }
 
+virDomainXMLPrivateDataCallbacks virCHDriverPrivateDataCallbacks = {
+.alloc = virCHDomainObjPrivateAlloc,
+.free = virCHDomainObjPrivateFree,
+.vcpuNew = virCHDomainVcpuPrivateNew,
+};
+
 static int
 chValidateDomainDeviceDef(const virDomainDeviceDef *dev,
   const virDomainDef *def G_GNUC_UNUSED,
diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h
index 04d19398b4..75b9933130 100644
--- a/src/ch/ch_domain.h
+++ b/src/ch/ch_domain.h
@@ -62,6 +62,17 @@ struct _virCHDomainObjPrivate {
 
 virCHMonitor *virCHDomainGetMonitor(virDomainObj *vm);
 
+typedef struct _virCHDomainVcpuPrivate virCHDomainVcpuPrivate;
+struct _virCHDomainVcpuPrivate {
+virObject parent;
+
+pid_t tid; /* vcpu thread id */
+virTristateBool halted;
+};
+
+#define CH_DOMAIN_VCPU_PRIVATE(vcpu) \
+((virCHDomainVcpuPrivate *) (vcpu)->privateData)
+
 extern virDomainXMLPrivateDataCallbacks virCHDriverPrivateDataCallbacks;
 extern virDomainDefParserConfig virCHDriverDomainDefParserConfig;
 
-- 
2.27.0

[libvirt PATCH 05/13] ch_driver, ch_domain: vcpu info getter callbacks

[Vineeth Pillai]

Signed-off-by: Vineeth Pillai 
Signed-off-by: Praveen K Paladugu 
---
 src/ch/ch_domain.c |  25 
 src/ch/ch_domain.h |   4 ++
 src/ch/ch_driver.c | 138 +
 3 files changed, 167 insertions(+)

diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c
index fedde4581b..c0b0b1005a 100644
--- a/src/ch/ch_domain.c
+++ b/src/ch/ch_domain.c
@@ -338,3 +338,28 @@ virCHDomainGetMonitor(virDomainObj *vm)
 {
 return CH_DOMAIN_PRIVATE(vm)->monitor;
 }
+
+pid_t
+virCHDomainGetVcpuPid(virDomainObj *vm,
+ unsigned int vcpuid)
+{
+virDomainVcpuDef *vcpu = virDomainDefGetVcpu(vm->def, vcpuid);
+return CH_DOMAIN_VCPU_PRIVATE(vcpu)->tid;
+}
+
+bool
+virCHDomainHasVcpuPids(virDomainObj *vm)
+{
+size_t i;
+size_t maxvcpus = virDomainDefGetVcpusMax(vm->def);
+virDomainVcpuDef *vcpu;
+
+for (i = 0; i < maxvcpus; i++) {
+vcpu = virDomainDefGetVcpu(vm->def, i);
+
+if (CH_DOMAIN_VCPU_PRIVATE(vcpu)->tid > 0)
+return true;
+}
+
+return false;
+}
diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h
index 75b9933130..d9c9d34a19 100644
--- a/src/ch/ch_domain.h
+++ b/src/ch/ch_domain.h
@@ -82,3 +82,7 @@ virCHDomainObjBeginJob(virDomainObj *obj, enum virCHDomainJob 
job)
 
 void
 virCHDomainObjEndJob(virDomainObj *obj);
+
+int virCHDomainRefreshVcpuInfo(virDomainObj *vm);
+pid_t virCHDomainGetVcpuPid(virDomainObj *vm, unsigned int vcpuid);
+bool virCHDomainHasVcpuPids(virDomainObj *vm);
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 1824d2fd16..8ea5ce393d 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -952,6 +952,141 @@ static int chStateInitialize(bool privileged,
 return ret;
 }
 
+static int
+chDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags)
+{
+virDomainObj *vm;
+virDomainDef *def;
+int ret = -1;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_DOMAIN_AFFECT_CONFIG |
+  VIR_DOMAIN_VCPU_MAXIMUM |
+  VIR_DOMAIN_VCPU_GUEST, -1);
+
+if (!(vm = chDomObjFromDomain(dom)))
+return -1;
+
+if (virDomainGetVcpusFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
+goto cleanup;
+
+if (!(def = virDomainObjGetOneDef(vm, flags)))
+goto cleanup;
+
+if (flags & VIR_DOMAIN_VCPU_MAXIMUM)
+ret = virDomainDefGetVcpusMax(def);
+else
+ret = virDomainDefGetVcpus(def);
+
+
+cleanup:
+virDomainObjEndAPI();
+return ret;
+}
+
+static int
+chDomainGetMaxVcpus(virDomainPtr dom)
+{
+return chDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE | 
VIR_DOMAIN_VCPU_MAXIMUM));
+}
+
+static int
+chDomainHelperGetVcpus(virDomainObj *vm,
+   virVcpuInfoPtr info,
+   unsigned long long *cpuwait,
+   int maxinfo,
+   unsigned char *cpumaps,
+   int maplen)
+{
+size_t ncpuinfo = 0;
+size_t i;
+
+if (maxinfo == 0)
+return 0;
+
+if (!virCHDomainHasVcpuPids(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+   "%s", _("cpu affinity is not supported"));
+return -1;
+}
+
+if (info)
+memset(info, 0, sizeof(*info) * maxinfo);
+
+if (cpumaps)
+memset(cpumaps, 0, sizeof(*cpumaps) * maxinfo);
+
+for (i = 0; i < virDomainDefGetVcpusMax(vm->def) && ncpuinfo < maxinfo; 
i++) {
+virDomainVcpuDef *vcpu = virDomainDefGetVcpu(vm->def, i);
+pid_t vcpupid = virCHDomainGetVcpuPid(vm, i);
+virVcpuInfoPtr vcpuinfo = info + ncpuinfo;
+
+if (!vcpu->online)
+continue;
+
+if (info) {
+vcpuinfo->number = i;
+vcpuinfo->state = VIR_VCPU_RUNNING;
+if (virProcessGetStatInfo(>cpuTime,
+  >cpu, NULL,
+  vm->pid, vcpupid) < 0) {
+virReportSystemError(errno, "%s",
+ _("cannot get vCPU placement & pCPU 
time"));
+return -1;
+}
+}
+
+if (cpumaps) {
+unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, ncpuinfo);
+virBitmap *map = NULL;
+
+if (!(map = virProcessGetAffinity(vcpupid)))
+return -1;
+
+virBitmapToDataBuf(map, cpumap, maplen);
+virBitmapFree(map);
+}
+
+if (cpuwait) {
+if (virProcessGetSchedInfo(&(cpuwait[ncpuinfo]), vm->pid, vcpupid) 
< 0)
+return -1;
+}
+
+ncpuinfo++;
+}
+
+return ncpuinfo;
+}
+
+static int
+chDomainGetVcpus(virDomainPtr dom,
+   virVcpuInfoPtr info,
+   int maxinfo,
+   unsigned char *cpumaps,
+   int maplen)
+{
+virDomainObj *vm;
+int ret = -1;
+
+if (!(vm = chDomObjFromDomain(dom)))
+goto cleanup;
+
+if 

[libvirt PATCH 02/13] ch: Explicitly link to virt_util_lib

[Vineeth Pillai]

link to virt_util_lib while building ch driver.

Signed-off-by: Vineeth Pillai 
---
 src/ch/meson.build | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/ch/meson.build b/src/ch/meson.build
index e34974d56c..5c6cab2a9f 100644
--- a/src/ch/meson.build
+++ b/src/ch/meson.build
@@ -30,6 +30,9 @@ if conf.has('WITH_CH')
 include_directories: [
   conf_inc_dir,
 ],
+link_with: [
+  virt_util_lib,
+]
   )
 
   virt_modules += {
-- 
2.27.0

[libvirt PATCH 02/13] ch: Explicitly link to virt_util_lib

[Vineeth Pillai]

link to virt_util_lib while building ch driver.

Signed-off-by: Vineeth Pillai 
---
 src/ch/meson.build | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/ch/meson.build b/src/ch/meson.build
index e34974d56c..5c6cab2a9f 100644
--- a/src/ch/meson.build
+++ b/src/ch/meson.build
@@ -30,6 +30,9 @@ if conf.has('WITH_CH')
 include_directories: [
   conf_inc_dir,
 ],
+link_with: [
+  virt_util_lib,
+]
   )
 
   virt_modules += {
-- 
2.27.0

[libvirt PATCH 01/13] util: Helper functions to get process info

[Vineeth Pillai]

These helper methods are used to capture vcpu information
in ch driver.

Signed-off-by: Vineeth Pillai 
Signed-off-by: Praveen K Paladugu 
---
 src/util/virprocess.c | 136 ++
 src/util/virprocess.h |   5 ++
 2 files changed, 141 insertions(+)

diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 6de3f36f52..0164d70df6 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -1721,3 +1721,139 @@ virProcessSetScheduler(pid_t pid G_GNUC_UNUSED,
 }
 
 #endif /* !WITH_SCHED_SETSCHEDULER */
+
+/*
+TODO: This method was cloned from qemuGetProcessInfo in src/qemu/qemu_driver.c.
+Need to refactor qemu driver to use this shared function.
+*/
+int
+virProcessGetStatInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss,
+   pid_t pid, pid_t tid)
+{
+g_autofree char *proc = NULL;
+FILE *pidinfo;
+unsigned long long usertime = 0, systime = 0;
+long rss = 0;
+int cpu = 0;
+
+/* In general, we cannot assume pid_t fits in int; but /proc parsing
+ * is specific to Linux where int works fine.  */
+if (tid)
+proc = g_strdup_printf("/proc/%d/task/%d/stat", (int)pid, tid);
+else
+proc = g_strdup_printf("/proc/%d/stat", (int)pid);
+if (!proc)
+return -1;
+
+pidinfo = fopen(proc, "r");
+
+/* See 'man proc' for information about what all these fields are. We're
+ * only interested in a very few of them */
+if (!pidinfo ||
+fscanf(pidinfo,
+   /* pid -> stime */
+   "%*d (%*[^)]) %*c %*d %*d %*d %*d %*d %*u %*u %*u %*u %*u %llu 
%llu"
+   /* cutime -> endcode */
+   "%*d %*d %*d %*d %*d %*d %*u %*u %ld %*u %*u %*u"
+   /* startstack -> processor */
+   "%*u %*u %*u %*u %*u %*u %*u %*u %*u %*u %*d %d",
+   , , , ) != 4) {
+VIR_WARN("cannot parse process status data");
+}
+
+/* We got jiffies
+ * We want nanoseconds
+ * _SC_CLK_TCK is jiffies per second
+ * So calculate thus
+ */
+if (cpuTime)
+*cpuTime = 1000ull * 1000ull * 1000ull * (usertime + systime)
+/ (unsigned long long)sysconf(_SC_CLK_TCK);
+if (lastCpu)
+*lastCpu = cpu;
+
+if (vm_rss)
+*vm_rss = rss * virGetSystemPageSizeKB();
+
+
+VIR_DEBUG("Got status for %d/%d user=%llu sys=%llu cpu=%d rss=%ld",
+  (int)pid, tid, usertime, systime, cpu, rss);
+
+VIR_FORCE_FCLOSE(pidinfo);
+
+return 0;
+}
+/*
+TODO: This method was cloned from qemuGetSchedInfo in src/qemu/qemu_driver.c.
+Need to refactor qemu driver to use this shared function.
+*/
+int virProcessGetSchedInfo(unsigned long long *cpuWait, pid_t pid, pid_t tid)
+{
+g_autofree char *proc = NULL;
+g_autofree char *data = NULL;
+char **lines = NULL;
+size_t i;
+int ret = -1;
+double val;
+
+*cpuWait = 0;
+
+/* In general, we cannot assume pid_t fits in int; but /proc parsing
+ * is specific to Linux where int works fine.  */
+if (tid)
+proc = g_strdup_printf("/proc/%d/task/%d/sched", (int)pid, (int)tid);
+else
+proc = g_strdup_printf("/proc/%d/sched", (int)pid);
+if (!proc)
+goto cleanup;
+ret = -1;
+
+/* The file is not guaranteed to exist (needs CONFIG_SCHED_DEBUG) */
+if (access(proc, R_OK) < 0) {
+ret = 0;
+goto cleanup;
+}
+
+if (virFileReadAll(proc, (1<<16), ) < 0)
+goto cleanup;
+
+lines = g_strsplit(data, "\n", 0);
+if (!lines)
+goto cleanup;
+
+for (i = 0; lines[i] != NULL; i++) {
+const char *line = lines[i];
+
+/* Needs CONFIG_SCHEDSTATS. The second check
+ * is the old name the kernel used in past */
+if (STRPREFIX(line, "se.statistics.wait_sum") ||
+STRPREFIX(line, "se.wait_sum")) {
+line = strchr(line, ':');
+if (!line) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Missing separator in sched info '%s'"),
+   lines[i]);
+goto cleanup;
+}
+line++;
+while (*line == ' ')
+line++;
+
+if (virStrToDouble(line, NULL, ) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Unable to parse sched info value '%s'"),
+   line);
+goto cleanup;
+}
+
+*cpuWait = (unsigned long long)(val * 100);
+break;
+}
+}
+
+ret = 0;
+
+ cleanup:
+g_strfreev(lines);
+return ret;
+}
diff --git a/src/util/virprocess.h b/src/util/virprocess.h
index 9910331a0c..3a7c4c2d64 100644
--- a/src/util/virprocess.h
+++ b/src/util/virprocess.h
@@ -127,3 +127,8 @@ typedef enum {
 } virProcessNamespaceFlags;
 
 int virProcessNamespaceAvailable(unsigned int ns);
+
+int virProcessGetStatInfo(unsigned 

[libvirt PATCH 06/13] ch_driver: domainGetVcpuPinInfo and nodeGetCPUMap

[Vineeth Pillai]

Add domainGetVcpuPinInfo and nodeGetCPUMap callbacks to ch driver

Signed-off-by: Vineeth Pillai 
Signed-off-by: Praveen K Paladugu 
---
 src/ch/ch_domain.h | 12 +--
 src/ch/ch_driver.c | 54 ++
 2 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h
index d9c9d34a19..e35777a9ec 100644
--- a/src/ch/ch_domain.h
+++ b/src/ch/ch_domain.h
@@ -23,6 +23,7 @@
 #include "ch_conf.h"
 #include "ch_monitor.h"
 #include "virchrdev.h"
+#include "vircgroup.h"
 
 /* Give up waiting for mutex after 30 seconds */
 #define CH_JOB_WAIT_TIME (1000ull * 30)
@@ -52,9 +53,16 @@ typedef struct _virCHDomainObjPrivate virCHDomainObjPrivate;
 struct _virCHDomainObjPrivate {
 struct virCHDomainJobObj job;
 
-virCHMonitor *monitor;
+virChrdevs *chrdevs;
+
+virCgroup *cgroup;
 
- virChrdevs *chrdevs;
+virCHDriver *driver;
+virCHMonitor *monitor;
+char *machineName;
+virBitmap *autoNodeset;
+virBitmap *autoCpuset;
+virChrdevs *devs;
 };
 
 #define CH_DOMAIN_PRIVATE(vm) \
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 8ea5ce393d..62ca6c1994 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -990,6 +990,58 @@ chDomainGetMaxVcpus(virDomainPtr dom)
 return chDomainGetVcpusFlags(dom, (VIR_DOMAIN_AFFECT_LIVE | 
VIR_DOMAIN_VCPU_MAXIMUM));
 }
 
+static int
+chDomainGetVcpuPinInfo(virDomain *dom,
+   int ncpumaps,
+   unsigned char *cpumaps,
+   int maplen,
+   unsigned int flags)
+{
+virDomainObj *vm = NULL;
+virDomainDef *def;
+bool live;
+int ret = -1;
+g_autoptr(virBitmap) hostcpus = NULL;
+virBitmap *autoCpuset = NULL;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+if (!(vm = chDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainGetVcpuPinInfoEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (!(def = virDomainObjGetOneDefState(vm, flags, )))
+goto cleanup;
+
+if (!(hostcpus = virHostCPUGetAvailableCPUsBitmap()))
+goto cleanup;
+
+if (live)
+autoCpuset = CH_DOMAIN_PRIVATE(vm)->autoCpuset;
+
+ret = virDomainDefGetVcpuPinInfoHelper(def, maplen, ncpumaps, cpumaps,
+   hostcpus, autoCpuset);
+cleanup:
+virDomainObjEndAPI();
+return ret;
+}
+
+static int
+chNodeGetCPUMap(virConnectPtr conn,
+  unsigned char **cpumap,
+  unsigned int *online,
+  unsigned int flags)
+{
+if (virNodeGetCPUMapEnsureACL(conn) < 0)
+return -1;
+
+return virHostCPUGetMap(cpumap, online, flags);
+}
+
+
 static int
 chDomainHelperGetVcpus(virDomainObj *vm,
virVcpuInfoPtr info,
@@ -1126,6 +1178,8 @@ static virHypervisorDriver chHypervisorDriver = {
 .domainGetVcpus = chDomainGetVcpus, /* 7.9.0 */
 .domainGetVcpusFlags = chDomainGetVcpusFlags,   /* 7.9.0 */
 .domainGetMaxVcpus = chDomainGetMaxVcpus,   /* 7.9.0 */
+.domainGetVcpuPinInfo = chDomainGetVcpuPinInfo, /* 7.9.0 */
+.nodeGetCPUMap = chNodeGetCPUMap,   /* 7.9.0 */
 };
 
 static virConnectDriver chConnectDriver = {
-- 
2.27.0

[libvirt PATCH 11/13] ch_driver: add numatune callbacks for CH driver

[Vineeth Pillai]

Signed-off-by: Vineeth Pillai 
Signed-off-by: Praveen K Paladugu 
---
 src/ch/ch_driver.c | 278 +
 1 file changed, 278 insertions(+)

diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 9ad23c1710..97dfda85e1 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -43,6 +43,7 @@
 #include "viruri.h"
 #include "virutil.h"
 #include "viruuid.h"
+#include "virnuma.h"
 
 #define VIR_FROM_THIS VIR_FROM_CH
 
@@ -1320,6 +1321,281 @@ chDomainPinVcpu(virDomainPtr dom,
   VIR_DOMAIN_AFFECT_LIVE);
 }
 
+#define CH_NB_NUMA_PARAM 2
+
+static int
+chDomainGetNumaParameters(virDomainPtr dom,
+  virTypedParameterPtr params,
+  int *nparams,
+  unsigned int flags)
+{
+size_t i;
+virDomainObj *vm = NULL;
+virDomainNumatuneMemMode tmpmode = VIR_DOMAIN_NUMATUNE_MEM_STRICT;
+virCHDomainObjPrivate *priv;
+g_autofree char *nodeset = NULL;
+int ret = -1;
+virDomainDef *def = NULL;
+bool live = false;
+virBitmap *autoNodeset = NULL;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_DOMAIN_AFFECT_CONFIG |
+  VIR_TYPED_PARAM_STRING_OKAY, -1);
+
+if (!(vm = virCHDomainObjFromDomain(dom)))
+return -1;
+priv = vm->privateData;
+
+if (virDomainGetNumaParametersEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (!(def = virDomainObjGetOneDefState(vm, flags, )))
+goto cleanup;
+
+if (live)
+autoNodeset = priv->autoNodeset;
+
+if ((*nparams) == 0) {
+*nparams = CH_NB_NUMA_PARAM;
+ret = 0;
+goto cleanup;
+}
+
+for (i = 0; i < CH_NB_NUMA_PARAM && i < *nparams; i++) {
+virMemoryParameterPtr param = [i];
+
+switch (i) {
+case 0: /* fill numa mode here */
+ignore_value(virDomainNumatuneGetMode(def->numa, -1, ));
+
+if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_MODE,
+VIR_TYPED_PARAM_INT, tmpmode) < 0)
+goto cleanup;
+
+break;
+
+case 1: /* fill numa nodeset here */
+nodeset = virDomainNumatuneFormatNodeset(def->numa, autoNodeset, 
-1);
+
+if (!nodeset ||
+virTypedParameterAssign(param, VIR_DOMAIN_NUMA_NODESET,
+VIR_TYPED_PARAM_STRING, nodeset) < 0)
+goto cleanup;
+
+nodeset = NULL;
+break;
+
+/* coverity[dead_error_begin] */
+default:
+break;
+/* should not hit here */
+}
+}
+
+if (*nparams > CH_NB_NUMA_PARAM)
+*nparams = CH_NB_NUMA_PARAM;
+ret = 0;
+
+ cleanup:
+virDomainObjEndAPI();
+return ret;
+}
+
+static int
+chDomainSetNumaParamsLive(virDomainObj *vm,
+  virBitmap *nodeset)
+{
+virCgroup *cgroup_temp = NULL;
+virCHDomainObjPrivate *priv = vm->privateData;
+g_autofree char *nodeset_str = NULL;
+virDomainNumatuneMemMode mode;
+size_t i = 0;
+int ret = -1;
+
+if (virDomainNumatuneGetMode(vm->def->numa, -1, ) == 0 &&
+mode != VIR_DOMAIN_NUMATUNE_MEM_STRICT) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("change of nodeset for running domain "
+ "requires strict numa mode"));
+goto cleanup;
+}
+
+if (!virNumaNodesetIsAvailable(nodeset))
+goto cleanup;
+
+/* Ensure the cpuset string is formatted before passing to cgroup */
+if (!(nodeset_str = virBitmapFormat(nodeset)))
+goto cleanup;
+
+if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0,
+   false, _temp) < 0 ||
+virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0)
+goto cleanup;
+virCgroupFree(cgroup_temp);
+
+for (i = 0; i < virDomainDefGetVcpusMax(vm->def); i++) {
+virDomainVcpuDef *vcpu = virDomainDefGetVcpu(vm->def, i);
+
+if (!vcpu->online)
+continue;
+
+if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, i,
+   false, _temp) < 0 ||
+virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0)
+goto cleanup;
+virCgroupFree(cgroup_temp);
+}
+
+for (i = 0; i < vm->def->niothreadids; i++) {
+if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD,
+   vm->def->iothreadids[i]->iothread_id,
+   false, _temp) < 0 ||
+virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0)
+goto cleanup;
+virCgroupFree(cgroup_temp);
+}
+
+/* set nodeset for root cgroup */
+if (virCgroupSetCpusetMems(priv->cgroup, nodeset_str) < 0)
+goto cleanup;
+
+ret = 0;
+ cleanup:
+virCgroupFree(cgroup_temp);
+
+return ret;

[libvirt PATCH 00/13] cgroup and thread management in ch driver.

[Vineeth Pillai]

This patchset adds support for cgroup management of ch threads. This version
correctly manages cgroups for vcpu and emulator threads created by ch. cgroup
management for iothreads is not yet supported.

Along with cgroup management, this patchset also enables support for pinning
vcpu and emulator threads to selected host cpus.

Praveen K Paladugu (2):
  ch_process: Setup emulator and iothread settings
  ch_driver: emulator threadinfo & pinning callbacks

Vineeth Pillai (11):
  util: Helper functions to get process info
  ch: Explicitly link to virt_util_lib
  ch_domain: add virCHDomainGetMonitor helper method
  ch_domain: add methods to manage private vcpu data
  ch_driver,ch_domain: vcpu info getter callbacks
  ch_driver: domainGetVcpuPinInfo and nodeGetCPUMap
  ch_monitor: Get nicindexes in prep for cgroup mgmt
  ch_cgroup: methods for cgroup mgmt in ch driver
  ch_driver,ch_domain: vcpupin callback in ch driver
  ch_driver: enable typed param string for numatune
  ch_driver: add numatune callbacks for CH driver

 po/POTFILES.in|   1 +
 src/ch/ch_cgroup.c| 457 
 src/ch/ch_cgroup.h|  45 +++
 src/ch/ch_conf.c  |   2 +
 src/ch/ch_conf.h  |   9 +-
 src/ch/ch_domain.c| 170 -
 src/ch/ch_domain.h|  32 +-
 src/ch/ch_driver.c| 810 +-
 src/ch/ch_monitor.c   | 254 -
 src/ch/ch_monitor.h   |  60 +++-
 src/ch/ch_process.c   | 368 ++-
 src/ch/ch_process.h   |   3 +
 src/ch/meson.build|   6 +
 src/util/virprocess.c | 136 +++
 src/util/virprocess.h |   5 +
 15 files changed, 2329 insertions(+), 29 deletions(-)
 create mode 100644 src/ch/ch_cgroup.c
 create mode 100644 src/ch/ch_cgroup.h

-- 
2.27.0

[libvirt PATCH 04/13] ch_domain: add methods to manage private vcpu data

[Vineeth Pillai]

Signed-off-by: Vineeth Pillai 
Signed-off-by: Praveen K Paladugu 
---
 src/ch/ch_domain.c | 50 +-
 src/ch/ch_domain.h | 11 ++
 2 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c
index ae79b47253..fedde4581b 100644
--- a/src/ch/ch_domain.c
+++ b/src/ch/ch_domain.c
@@ -166,11 +166,6 @@ virCHDomainObjPrivateFree(void *data)
 g_free(priv);
 }
 
-virDomainXMLPrivateDataCallbacks virCHDriverPrivateDataCallbacks = {
-.alloc = virCHDomainObjPrivateAlloc,
-.free = virCHDomainObjPrivateFree,
-};
-
 static int
 virCHDomainDefPostParseBasic(virDomainDef *def,
  void *opaque G_GNUC_UNUSED)
@@ -187,6 +182,45 @@ virCHDomainDefPostParseBasic(virDomainDef *def,
 return 0;
 }
 
+static virClass *virCHDomainVcpuPrivateClass;
+static void virCHDomainVcpuPrivateDispose(void *obj);
+
+static int
+virCHDomainVcpuPrivateOnceInit(void)
+{
+if (!VIR_CLASS_NEW(virCHDomainVcpuPrivate, virClassForObject()))
+return -1;
+
+return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(virCHDomainVcpuPrivate);
+
+static virObject *
+virCHDomainVcpuPrivateNew(void)
+{
+virCHDomainVcpuPrivate *priv;
+
+if (virCHDomainVcpuPrivateInitialize() < 0)
+return NULL;
+
+if (!(priv = virObjectNew(virCHDomainVcpuPrivateClass)))
+return NULL;
+
+return (virObject *) priv;
+}
+
+
+static void
+virCHDomainVcpuPrivateDispose(void *obj)
+{
+virCHDomainVcpuPrivate *priv = obj;
+
+priv->tid = 0;
+
+return;
+}
+
 static int
 virCHDomainDefPostParse(virDomainDef *def,
 unsigned int parseFlags G_GNUC_UNUSED,
@@ -205,6 +239,12 @@ virCHDomainDefPostParse(virDomainDef *def,
 return 0;
 }
 
+virDomainXMLPrivateDataCallbacks virCHDriverPrivateDataCallbacks = {
+.alloc = virCHDomainObjPrivateAlloc,
+.free = virCHDomainObjPrivateFree,
+.vcpuNew = virCHDomainVcpuPrivateNew,
+};
+
 static int
 chValidateDomainDeviceDef(const virDomainDeviceDef *dev,
   const virDomainDef *def G_GNUC_UNUSED,
diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h
index 04d19398b4..75b9933130 100644
--- a/src/ch/ch_domain.h
+++ b/src/ch/ch_domain.h
@@ -62,6 +62,17 @@ struct _virCHDomainObjPrivate {
 
 virCHMonitor *virCHDomainGetMonitor(virDomainObj *vm);
 
+typedef struct _virCHDomainVcpuPrivate virCHDomainVcpuPrivate;
+struct _virCHDomainVcpuPrivate {
+virObject parent;
+
+pid_t tid; /* vcpu thread id */
+virTristateBool halted;
+};
+
+#define CH_DOMAIN_VCPU_PRIVATE(vcpu) \
+((virCHDomainVcpuPrivate *) (vcpu)->privateData)
+
 extern virDomainXMLPrivateDataCallbacks virCHDriverPrivateDataCallbacks;
 extern virDomainDefParserConfig virCHDriverDomainDefParserConfig;
 
-- 
2.27.0

[libvirt PATCH 08/13] ch_cgroup: methods for cgroup mgmt in ch driver

[Vineeth Pillai]

Signed-off-by: Vineeth Pillai 
Signed-off-by: Praveen K Paladugu 
---
 po/POTFILES.in  |   1 +
 src/ch/ch_cgroup.c  | 457 
 src/ch/ch_cgroup.h  |  45 +
 src/ch/ch_conf.c|   2 +
 src/ch/ch_conf.h|   4 +-
 src/ch/ch_domain.c  |  33 
 src/ch/ch_domain.h  |   3 +-
 src/ch/ch_monitor.c | 125 ++--
 src/ch/ch_monitor.h |  54 +-
 src/ch/ch_process.c | 288 +++-
 src/ch/ch_process.h |   3 +
 src/ch/meson.build  |   2 +
 12 files changed, 991 insertions(+), 26 deletions(-)
 create mode 100644 src/ch/ch_cgroup.c
 create mode 100644 src/ch/ch_cgroup.h

diff --git a/po/POTFILES.in b/po/POTFILES.in
index b554cf08ca..3a8db501bc 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -19,6 +19,7 @@
 @SRCDIR@src/bhyve/bhyve_parse_command.c
 @SRCDIR@src/bhyve/bhyve_process.c
 @SRCDIR@src/ch/ch_conf.c
+@SRCDIR@src/ch/ch_cgroup.c
 @SRCDIR@src/ch/ch_domain.c
 @SRCDIR@src/ch/ch_driver.c
 @SRCDIR@src/ch/ch_monitor.c
diff --git a/src/ch/ch_cgroup.c b/src/ch/ch_cgroup.c
new file mode 100644
index 00..6be2184cf1
--- /dev/null
+++ b/src/ch/ch_cgroup.c
@@ -0,0 +1,457 @@
+/*
+ * ch_cgroup.c: CH cgroup management
+ *
+ * Copyright Microsoft Corp. 2020-2021
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ */
+
+#include 
+
+#include "ch_cgroup.h"
+#include "ch_domain.h"
+#include "ch_process.h"
+#include "vircgroup.h"
+#include "virlog.h"
+#include "viralloc.h"
+#include "virerror.h"
+#include "domain_audit.h"
+#include "domain_cgroup.h"
+#include "virscsi.h"
+#include "virstring.h"
+#include "virfile.h"
+#include "virtypedparam.h"
+#include "virnuma.h"
+#include "virdevmapper.h"
+#include "virutil.h"
+
+#define VIR_FROM_THIS VIR_FROM_CH
+
+VIR_LOG_INIT("ch.ch_cgroup");
+
+static int
+chSetupBlkioCgroup(virDomainObj * vm)
+{
+virCHDomainObjPrivate *priv = vm->privateData;
+
+if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_BLKIO)) {
+if (vm->def->blkio.weight || vm->def->blkio.ndevices) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("Block I/O tuning is not available on this 
host"));
+return -1;
+} else {
+return 0;
+}
+}
+
+return virDomainCgroupSetupBlkio(priv->cgroup, vm->def->blkio);
+}
+
+
+static int
+chSetupMemoryCgroup(virDomainObj * vm)
+{
+virCHDomainObjPrivate *priv = vm->privateData;
+
+if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_MEMORY)) {
+if (virMemoryLimitIsSet(vm->def->mem.hard_limit) ||
+virMemoryLimitIsSet(vm->def->mem.soft_limit) ||
+virMemoryLimitIsSet(vm->def->mem.swap_hard_limit)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("Memory cgroup is not available on this host"));
+return -1;
+} else {
+return 0;
+}
+}
+
+return virDomainCgroupSetupMemtune(priv->cgroup, vm->def->mem);
+}
+
+static int
+chSetupCpusetCgroup(virDomainObj * vm)
+{
+virCHDomainObjPrivate *priv = vm->privateData;
+
+if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET))
+return 0;
+
+if (virCgroupSetCpusetMemoryMigrate(priv->cgroup, true) < 0)
+return -1;
+
+return 0;
+}
+
+
+static int
+chSetupCpuCgroup(virDomainObj * vm)
+{
+virCHDomainObjPrivate *priv = vm->privateData;
+
+if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) {
+if (vm->def->cputune.sharesSpecified) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("CPU tuning is not available on this host"));
+return -1;
+} else {
+return 0;
+}
+}
+
+if (vm->def->cputune.sharesSpecified) {
+
+if (virCgroupSetCpuShares(priv->cgroup, vm->def->cputune.shares) < 0)
+return -1;
+
+}
+
+return 0;
+}
+
+
+static int
+chInitCgroup(virDomainObj * vm, size_t nnicindexes, int *nicindexes)
+{
+virCHDomainObjPrivate *priv = vm->privateData;
+
+g_autoptr(virCHDriverConfig) cfg = virCHDriverGetConfig(priv->driver);
+
+if (!priv->driver->privileged)
+return 0;
+
+if (!virCgroupAvailable())
+return 0;
+
+virCgroupFree(priv->cgroup);
+
+if (!vm->def->resource) 

[libvirt PATCH 07/13] ch_monitor: Get nicindexes in prep for cgroup mgmt

[Vineeth Pillai]

Signed-off-by: Vineeth Pillai 
Signed-off-by: Praveen Paladugu 
---
 src/ch/ch_conf.h|  5 +++
 src/ch/ch_domain.c  | 26 +-
 src/ch/ch_domain.h  |  4 +--
 src/ch/ch_driver.c  |  4 ++-
 src/ch/ch_monitor.c | 85 +
 src/ch/ch_monitor.h | 14 +++-
 src/ch/ch_process.c |  6 +++-
 src/ch/meson.build  |  1 +
 8 files changed, 132 insertions(+), 13 deletions(-)

diff --git a/src/ch/ch_conf.h b/src/ch/ch_conf.h
index 37c36d9a09..49f286f97a 100644
--- a/src/ch/ch_conf.h
+++ b/src/ch/ch_conf.h
@@ -44,6 +44,11 @@ struct _virCHDriver
 {
 virMutex lock;
 
+bool privileged;
+
+/* Embedded Mode: Not yet supported */
+char *embeddedRoot;
+
 /* Require lock to get a reference on the object,
  * lockless access thereafter */
 virCaps *caps;
diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c
index c0b0b1005a..e1030800aa 100644
--- a/src/ch/ch_domain.c
+++ b/src/ch/ch_domain.c
@@ -21,10 +21,12 @@
 #include 
 
 #include "ch_domain.h"
+#include "domain_driver.h"
 #include "viralloc.h"
 #include "virchrdev.h"
 #include "virlog.h"
 #include "virtime.h"
+#include "virsystemd.h"
 
 #define VIR_FROM_THIS VIR_FROM_CH
 
@@ -136,7 +138,7 @@ virCHDomainObjEndJob(virDomainObj *obj)
 }
 
 static void *
-virCHDomainObjPrivateAlloc(void *opaque G_GNUC_UNUSED)
+virCHDomainObjPrivateAlloc(void *opaque)
 {
 virCHDomainObjPrivate *priv;
 
@@ -152,6 +154,7 @@ virCHDomainObjPrivateAlloc(void *opaque G_GNUC_UNUSED)
 g_free(priv);
 return NULL;
 }
+priv->driver = opaque;
 
 return priv;
 }
@@ -363,3 +366,24 @@ virCHDomainHasVcpuPids(virDomainObj *vm)
 
 return false;
 }
+
+char *virCHDomainGetMachineName(virDomainObj *vm)
+{
+virCHDomainObjPrivate *priv = CH_DOMAIN_PRIVATE(vm);
+virCHDriver *driver = priv->driver;
+char *ret = NULL;
+
+if (vm->pid > 0) {
+ret = virSystemdGetMachineNameByPID(vm->pid);
+if (!ret)
+virResetLastError();
+}
+
+if (!ret)
+ret = virDomainDriverGenerateMachineName("ch",
+ driver->embeddedRoot,
+ vm->def->id, vm->def->name,
+ driver->privileged);
+
+return ret;
+}
diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h
index e35777a9ec..3ac3421015 100644
--- a/src/ch/ch_domain.h
+++ b/src/ch/ch_domain.h
@@ -54,9 +54,7 @@ struct _virCHDomainObjPrivate {
 struct virCHDomainJobObj job;
 
 virChrdevs *chrdevs;
-
 virCgroup *cgroup;
-
 virCHDriver *driver;
 virCHMonitor *monitor;
 char *machineName;
@@ -94,3 +92,5 @@ virCHDomainObjEndJob(virDomainObj *obj);
 int virCHDomainRefreshVcpuInfo(virDomainObj *vm);
 pid_t virCHDomainGetVcpuPid(virDomainObj *vm, unsigned int vcpuid);
 bool virCHDomainHasVcpuPids(virDomainObj *vm);
+
+char *virCHDomainGetMachineName(virDomainObj *vm);
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 62ca6c1994..ca854da123 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -944,7 +944,9 @@ static int chStateInitialize(bool privileged,
 goto cleanup;
 }
 
-ret = VIR_DRV_STATE_INIT_COMPLETE;
+ch_driver->privileged = privileged;
+
+return VIR_DRV_STATE_INIT_COMPLETE;
 
  cleanup:
 if (ret != VIR_DRV_STATE_INIT_COMPLETE)
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index 691d1ce64b..c0ae031200 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -228,7 +228,8 @@ virCHMonitorBuildDisksJson(virJSONValue *content, 
virDomainDef *vmdef)
 }
 
 static int
-virCHMonitorBuildNetJson(virJSONValue *nets, virDomainNetDef *netdef)
+virCHMonitorBuildNetJson(virJSONValue *nets, virDomainNetDef *netdef,
+size_t *nnicindexes, int **nicindexes)
 {
 virDomainNetType netType = virDomainNetGetActualType(netdef);
 char macaddr[VIR_MAC_STRING_BUFLEN];
@@ -263,6 +264,18 @@ virCHMonitorBuildNetJson(virJSONValue *nets, 
virDomainNetDef *netdef)
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("ethernet type supports a single guest ip"));
 }
+
+/* network and bridge use a tap device, and direct uses a
+ * macvtap device
+ */
+if (nicindexes && nnicindexes && netdef->ifname) {
+int nicindex = 0;
+if (virNetDevGetIndex(netdef->ifname, ) < 0)
+return -1;
+else
+VIR_APPEND_ELEMENT(*nicindexes, *nnicindexes, nicindex);
+}
+
 break;
 case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
 if ((virDomainChrType)netdef->data.vhostuser->type != 
VIR_DOMAIN_CHR_TYPE_UNIX) {
@@ -331,7 +344,8 @@ virCHMonitorBuildNetJson(virJSONValue *nets, 
virDomainNetDef *netdef)
 }
 
 static int
-virCHMonitorBuildNetsJson(virJSONValue *content, virDomainDef *vmdef)
+virCHMonitorBuildNetsJson(virJSONValue *content, virDomainDef *vmdef,
+   

[libvirt PATCH 11/13] ch_driver: add numatune callbacks for CH driver

[Vineeth Pillai]

Signed-off-by: Vineeth Pillai 
Signed-off-by: Praveen K Paladugu 
---
 src/ch/ch_driver.c | 278 +
 1 file changed, 278 insertions(+)

diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 9ad23c1710..97dfda85e1 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -43,6 +43,7 @@
 #include "viruri.h"
 #include "virutil.h"
 #include "viruuid.h"
+#include "virnuma.h"
 
 #define VIR_FROM_THIS VIR_FROM_CH
 
@@ -1320,6 +1321,281 @@ chDomainPinVcpu(virDomainPtr dom,
   VIR_DOMAIN_AFFECT_LIVE);
 }
 
+#define CH_NB_NUMA_PARAM 2
+
+static int
+chDomainGetNumaParameters(virDomainPtr dom,
+  virTypedParameterPtr params,
+  int *nparams,
+  unsigned int flags)
+{
+size_t i;
+virDomainObj *vm = NULL;
+virDomainNumatuneMemMode tmpmode = VIR_DOMAIN_NUMATUNE_MEM_STRICT;
+virCHDomainObjPrivate *priv;
+g_autofree char *nodeset = NULL;
+int ret = -1;
+virDomainDef *def = NULL;
+bool live = false;
+virBitmap *autoNodeset = NULL;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+  VIR_DOMAIN_AFFECT_CONFIG |
+  VIR_TYPED_PARAM_STRING_OKAY, -1);
+
+if (!(vm = virCHDomainObjFromDomain(dom)))
+return -1;
+priv = vm->privateData;
+
+if (virDomainGetNumaParametersEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (!(def = virDomainObjGetOneDefState(vm, flags, )))
+goto cleanup;
+
+if (live)
+autoNodeset = priv->autoNodeset;
+
+if ((*nparams) == 0) {
+*nparams = CH_NB_NUMA_PARAM;
+ret = 0;
+goto cleanup;
+}
+
+for (i = 0; i < CH_NB_NUMA_PARAM && i < *nparams; i++) {
+virMemoryParameterPtr param = [i];
+
+switch (i) {
+case 0: /* fill numa mode here */
+ignore_value(virDomainNumatuneGetMode(def->numa, -1, ));
+
+if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_MODE,
+VIR_TYPED_PARAM_INT, tmpmode) < 0)
+goto cleanup;
+
+break;
+
+case 1: /* fill numa nodeset here */
+nodeset = virDomainNumatuneFormatNodeset(def->numa, autoNodeset, 
-1);
+
+if (!nodeset ||
+virTypedParameterAssign(param, VIR_DOMAIN_NUMA_NODESET,
+VIR_TYPED_PARAM_STRING, nodeset) < 0)
+goto cleanup;
+
+nodeset = NULL;
+break;
+
+/* coverity[dead_error_begin] */
+default:
+break;
+/* should not hit here */
+}
+}
+
+if (*nparams > CH_NB_NUMA_PARAM)
+*nparams = CH_NB_NUMA_PARAM;
+ret = 0;
+
+ cleanup:
+virDomainObjEndAPI();
+return ret;
+}
+
+static int
+chDomainSetNumaParamsLive(virDomainObj *vm,
+  virBitmap *nodeset)
+{
+virCgroup *cgroup_temp = NULL;
+virCHDomainObjPrivate *priv = vm->privateData;
+g_autofree char *nodeset_str = NULL;
+virDomainNumatuneMemMode mode;
+size_t i = 0;
+int ret = -1;
+
+if (virDomainNumatuneGetMode(vm->def->numa, -1, ) == 0 &&
+mode != VIR_DOMAIN_NUMATUNE_MEM_STRICT) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("change of nodeset for running domain "
+ "requires strict numa mode"));
+goto cleanup;
+}
+
+if (!virNumaNodesetIsAvailable(nodeset))
+goto cleanup;
+
+/* Ensure the cpuset string is formatted before passing to cgroup */
+if (!(nodeset_str = virBitmapFormat(nodeset)))
+goto cleanup;
+
+if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0,
+   false, _temp) < 0 ||
+virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0)
+goto cleanup;
+virCgroupFree(cgroup_temp);
+
+for (i = 0; i < virDomainDefGetVcpusMax(vm->def); i++) {
+virDomainVcpuDef *vcpu = virDomainDefGetVcpu(vm->def, i);
+
+if (!vcpu->online)
+continue;
+
+if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, i,
+   false, _temp) < 0 ||
+virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0)
+goto cleanup;
+virCgroupFree(cgroup_temp);
+}
+
+for (i = 0; i < vm->def->niothreadids; i++) {
+if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD,
+   vm->def->iothreadids[i]->iothread_id,
+   false, _temp) < 0 ||
+virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0)
+goto cleanup;
+virCgroupFree(cgroup_temp);
+}
+
+/* set nodeset for root cgroup */
+if (virCgroupSetCpusetMems(priv->cgroup, nodeset_str) < 0)
+goto cleanup;
+
+ret = 0;
+ cleanup:
+virCgroupFree(cgroup_temp);
+
+return ret;

[libvirt PATCH 03/13] ch_domain: add virCHDomainGetMonitor helper method

[Vineeth Pillai]

Signed-off-by: Vineeth Pillai 
Signed-off-by: Praveen K Paladugu 
---
 src/ch/ch_domain.c | 6 ++
 src/ch/ch_domain.h | 5 +
 2 files changed, 11 insertions(+)

diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c
index 9d32d8669a..ae79b47253 100644
--- a/src/ch/ch_domain.c
+++ b/src/ch/ch_domain.c
@@ -292,3 +292,9 @@ virDomainDefParserConfig virCHDriverDomainDefParserConfig = 
{
 .domainPostParseCallback = virCHDomainDefPostParse,
 .deviceValidateCallback = chValidateDomainDeviceDef,
 };
+
+virCHMonitor *
+virCHDomainGetMonitor(virDomainObj *vm)
+{
+return CH_DOMAIN_PRIVATE(vm)->monitor;
+}
diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h
index 61b34b0467..04d19398b4 100644
--- a/src/ch/ch_domain.h
+++ b/src/ch/ch_domain.h
@@ -57,6 +57,11 @@ struct _virCHDomainObjPrivate {
  virChrdevs *chrdevs;
 };
 
+#define CH_DOMAIN_PRIVATE(vm) \
+((virCHDomainObjPrivate *) (vm)->privateData)
+
+virCHMonitor *virCHDomainGetMonitor(virDomainObj *vm);
+
 extern virDomainXMLPrivateDataCallbacks virCHDriverPrivateDataCallbacks;
 extern virDomainDefParserConfig virCHDriverDomainDefParserConfig;
 
-- 
2.27.0