Re: [libvirt PATCH 1/2] ch: use payload api to send kernel details

2023-10-20 Thread Michal Prívozník 
On 10/10/23 23:42, Praveen K Paladugu wrote:
> Starting with v28.0 cloud-hypervisor requires the use of "payload" api to pass
> kernel, initramfs and cmdline options. Extend ch driver to use the new
> api based on ch version.
> 
> Signed-off-by: Praveen K Paladugu 
> ---
>  src/ch/ch_capabilities.c | 55 
>  src/ch/ch_capabilities.h | 34 +
>  src/ch/ch_conf.h |  6 +
>  src/ch/ch_driver.c   |  3 +++
>  src/ch/ch_monitor.c  | 48 +++
>  src/ch/ch_monitor.h  |  4 ++-
>  src/ch/ch_process.c  |  2 +-
>  src/ch/meson.build   |  2 ++
>  8 files changed, 147 insertions(+), 7 deletions(-)
>  create mode 100644 src/ch/ch_capabilities.c
>  create mode 100644 src/ch/ch_capabilities.h
> 
> diff --git a/src/ch/ch_capabilities.c b/src/ch/ch_capabilities.c
> new file mode 100644
> index 00..b10485820c
> --- /dev/null
> +++ b/src/ch/ch_capabilities.c
> @@ -0,0 +1,55 @@
> +/*
> + * Copyright Microsoft Corp. 2023
> + *
> + * ch_capabilities.h: CH capabilities
> + *
> + * 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_capabilities.h"
> +
> +static void
> +virCHCapsSet(virBitmap *chCaps,
> +   virCHCapsFlags flag)
> +{
> +ignore_value(virBitmapSetBit(chCaps, flag));
> +}
> +
> +/**
> + * virCHCapsInitCHVersionCaps:
> + *
> + * Set all CH capabilities based on version of CH.
> + */
> +virBitmap *
> +virCHCapsInitCHVersionCaps(int version)
> +{
> +g_autoptr(virBitmap) chCaps = NULL;
> +chCaps = virBitmapNew(CH_CAPS_LAST);
> +
> +/* Version 28 deprecated kernel API:
> + * 
> https://github.com/cloud-hypervisor/cloud-hypervisor/releases/tag/v28.0
> + */
> +if (version >= 2800)
> +virCHCapsSet(chCaps, CH_KERNEL_API_DEPRCATED);
> +
> +
> +/* Starting Version 18, serial and console can be used in parallel */
> +if (version >= 1800)
> +virCHCapsSet(chCaps, CH_SERIAL_CONSOLE_IN_PARALLEL);

I wish there was a better way of checking for these facts. Historically,
we used to parse 'qemu -help' output which is a bit better than just
plain version check. OTOH, cloud hypervisor moves fast and probably
doesn't suffer the same problems as qemu, i.e. a lot of backports (even
features) with no version change. So I can live with this.

Michal

[libvirt PATCH 1/2] ch: use payload api to send kernel details

2023-10-10 Thread Praveen K Paladugu 
Starting with v28.0 cloud-hypervisor requires the use of "payload" api to pass
kernel, initramfs and cmdline options. Extend ch driver to use the new
api based on ch version.

Signed-off-by: Praveen K Paladugu 
---
 src/ch/ch_capabilities.c | 55 
 src/ch/ch_capabilities.h | 34 +
 src/ch/ch_conf.h |  6 +
 src/ch/ch_driver.c   |  3 +++
 src/ch/ch_monitor.c  | 48 +++
 src/ch/ch_monitor.h  |  4 ++-
 src/ch/ch_process.c  |  2 +-
 src/ch/meson.build   |  2 ++
 8 files changed, 147 insertions(+), 7 deletions(-)
 create mode 100644 src/ch/ch_capabilities.c
 create mode 100644 src/ch/ch_capabilities.h

diff --git a/src/ch/ch_capabilities.c b/src/ch/ch_capabilities.c
new file mode 100644
index 00..b10485820c
--- /dev/null
+++ b/src/ch/ch_capabilities.c
@@ -0,0 +1,55 @@
+/*
+ * Copyright Microsoft Corp. 2023
+ *
+ * ch_capabilities.h: CH capabilities
+ *
+ * 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_capabilities.h"
+
+static void
+virCHCapsSet(virBitmap *chCaps,
+   virCHCapsFlags flag)
+{
+ignore_value(virBitmapSetBit(chCaps, flag));
+}
+
+/**
+ * virCHCapsInitCHVersionCaps:
+ *
+ * Set all CH capabilities based on version of CH.
+ */
+virBitmap *
+virCHCapsInitCHVersionCaps(int version)
+{
+g_autoptr(virBitmap) chCaps = NULL;
+chCaps = virBitmapNew(CH_CAPS_LAST);
+
+/* Version 28 deprecated kernel API:
+ * https://github.com/cloud-hypervisor/cloud-hypervisor/releases/tag/v28.0
+ */
+if (version >= 2800)
+virCHCapsSet(chCaps, CH_KERNEL_API_DEPRCATED);
+
+
+/* Starting Version 18, serial and console can be used in parallel */
+if (version >= 1800)
+virCHCapsSet(chCaps, CH_SERIAL_CONSOLE_IN_PARALLEL);
+
+return g_steal_pointer(&chCaps);
+
+}
diff --git a/src/ch/ch_capabilities.h b/src/ch/ch_capabilities.h
new file mode 100644
index 00..703a6dbfe2
--- /dev/null
+++ b/src/ch/ch_capabilities.h
@@ -0,0 +1,34 @@
+/*
+ * Copyright Microsoft Corp. 2023
+ *
+ * ch_capabilities.h: CH capabilities
+ *
+ * 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
+ * .
+ */
+
+#pragma once
+#include "virbitmap.h"
+
+
+typedef enum {
+/* 0 */
+CH_KERNEL_API_DEPRCATED, /* Use `payload` in place of `kernel` api */
+CH_SERIAL_CONSOLE_IN_PARALLEL, /* Serial and Console ports can work in 
parallel */
+
+CH_CAPS_LAST /* this must always be the last item */
+} virCHCapsFlags;
+
+virBitmap *
+virCHCapsInitCHVersionCaps(int version);
diff --git a/src/ch/ch_conf.h b/src/ch/ch_conf.h
index b927621a97..5b9b42540d 100644
--- a/src/ch/ch_conf.h
+++ b/src/ch/ch_conf.h
@@ -22,6 +22,7 @@
 
 #include "virdomainobjlist.h"
 #include "virthread.h"
+#include "ch_capabilities.h"
 
 #define CH_DRIVER_NAME "CH"
 #define CH_CMD "cloud-hypervisor"
@@ -54,6 +55,11 @@ struct _virCHDriver
  * lockless access thereafter */
 virCaps *caps;
 
+/* Immutable pointer, Immutable object
+ * Initialized once and reused as needed
+ */
+virBitmap *chCaps;
+
 /* Immutable pointer, Immutable object */
 virDomainXMLOption *xmlopt;
 
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index b62645a8c8..bd271fc0ee 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -20,6 +20,7 @@
 
 #include 
 
+#include "ch_capabilities.h"
 #include "ch_conf.h"
 #include "ch_domain.h"
 #include "ch_driver.h"
@@ -899,6 +900,8 @@ static int chStateInitialize(bool privileged,
 goto cleanup;
 }
 
+ch_driver->chCaps = virCHCapsInitCHVersionCaps(ch_driver->version);
+
 ch_driver->privileged = privileged;
 ret = VIR_DRV_STATE_INIT_COMPLETE