[PATCH 00/13] CH: Add disk hotplug support to Cloud Hypervisor domains

Stefan Kober posted 13 patches 1 week, 2 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20250828125453.202358-1-stefan.kober@cyberus-technology.de
There is a newer version of this series
NEWS.rst            |   5 +
po/POTFILES         |   1 +
src/ch/ch_alias.c   |  59 ++++++++
src/ch/ch_alias.h   |  27 ++++
src/ch/ch_driver.c  |  86 ++++++++++++
src/ch/ch_hotplug.c | 333 ++++++++++++++++++++++++++++++++++++++++++++
src/ch/ch_hotplug.h |  33 +++++
src/ch/ch_monitor.c |  81 +++++++++--
src/ch/ch_monitor.h |   7 +
src/ch/ch_process.c |   4 +
src/ch/meson.build  |   4 +
11 files changed, 630 insertions(+), 10 deletions(-)
create mode 100644 src/ch/ch_alias.c
create mode 100644 src/ch/ch_alias.h
create mode 100644 src/ch/ch_hotplug.c
create mode 100644 src/ch/ch_hotplug.h
[PATCH 00/13] CH: Add disk hotplug support to Cloud Hypervisor domains
Posted by Stefan Kober 1 week, 2 days ago
This patchset adds the necessary functionality to support disk hotplugging in the CH driver.

Some alias handling that can be extended to other device types is added, in order to allow detaching the devices via the Cloud Hypervisor API.

Stefan Kober (13):
  ch: add DomainAttachDevice skeletons
  ch: add DomainDetachDevice skeletons
  ch: add ch_hotplug.{h,c} files to CH build
  ch: call into ch_hotplug implementation
  ch: pass disk alias to CHV
  ch: add ch_alias.{c,h} for device alias handling
  ch: add virCHMonitorPut function
  ch: use MonitorPut in MonitorPutNoContent
  ch: add disk attach functionality
  ch: assign aliases in ProcessPrepareDomain
  ch: Add virCHMonitorRemoveDevice function
  ch: implement disk device hot removal
  NEWS: announce disk hotplug support for ch

 NEWS.rst            |   5 +
 po/POTFILES         |   1 +
 src/ch/ch_alias.c   |  59 ++++++++
 src/ch/ch_alias.h   |  27 ++++
 src/ch/ch_driver.c  |  86 ++++++++++++
 src/ch/ch_hotplug.c | 333 ++++++++++++++++++++++++++++++++++++++++++++
 src/ch/ch_hotplug.h |  33 +++++
 src/ch/ch_monitor.c |  81 +++++++++--
 src/ch/ch_monitor.h |   7 +
 src/ch/ch_process.c |   4 +
 src/ch/meson.build  |   4 +
 11 files changed, 630 insertions(+), 10 deletions(-)
 create mode 100644 src/ch/ch_alias.c
 create mode 100644 src/ch/ch_alias.h
 create mode 100644 src/ch/ch_hotplug.c
 create mode 100644 src/ch/ch_hotplug.h

-- 
2.50.1
Re: [PATCH 00/13] CH: Add disk hotplug support to Cloud Hypervisor domains
Posted by Michal Prívozník via Devel 5 days, 13 hours ago
On 8/28/25 14:54, Stefan Kober wrote:
> This patchset adds the necessary functionality to support disk hotplugging in the CH driver.
> 
> Some alias handling that can be extended to other device types is added, in order to allow detaching the devices via the Cloud Hypervisor API.
> 
> Stefan Kober (13):
>   ch: add DomainAttachDevice skeletons
>   ch: add DomainDetachDevice skeletons
>   ch: add ch_hotplug.{h,c} files to CH build
>   ch: call into ch_hotplug implementation
>   ch: pass disk alias to CHV
>   ch: add ch_alias.{c,h} for device alias handling
>   ch: add virCHMonitorPut function
>   ch: use MonitorPut in MonitorPutNoContent
>   ch: add disk attach functionality
>   ch: assign aliases in ProcessPrepareDomain
>   ch: Add virCHMonitorRemoveDevice function
>   ch: implement disk device hot removal
>   NEWS: announce disk hotplug support for ch
> 
>  NEWS.rst            |   5 +
>  po/POTFILES         |   1 +
>  src/ch/ch_alias.c   |  59 ++++++++
>  src/ch/ch_alias.h   |  27 ++++
>  src/ch/ch_driver.c  |  86 ++++++++++++
>  src/ch/ch_hotplug.c | 333 ++++++++++++++++++++++++++++++++++++++++++++
>  src/ch/ch_hotplug.h |  33 +++++
>  src/ch/ch_monitor.c |  81 +++++++++--
>  src/ch/ch_monitor.h |   7 +
>  src/ch/ch_process.c |   4 +
>  src/ch/meson.build  |   4 +
>  11 files changed, 630 insertions(+), 10 deletions(-)
>  create mode 100644 src/ch/ch_alias.c
>  create mode 100644 src/ch/ch_alias.h
>  create mode 100644 src/ch/ch_hotplug.c
>  create mode 100644 src/ch/ch_hotplug.h
> 

I've taken closer look at these and in general they look good. I mean,
what Peter wrote still applies and needs to be addressed in v2, but
we're on a track here.

Michal
Re: [PATCH 00/13] CH: Add disk hotplug support to Cloud Hypervisor domains
Posted by Peter Krempa via Devel 1 week, 1 day ago
On Thu, Aug 28, 2025 at 14:54:40 +0200, Stefan Kober wrote:
> This patchset adds the necessary functionality to support disk hotplugging in the CH driver.
> 
> Some alias handling that can be extended to other device types is added, in order to allow detaching the devices via the Cloud Hypervisor API.
> 
> Stefan Kober (13):
>   ch: add DomainAttachDevice skeletons
>   ch: add DomainDetachDevice skeletons
>   ch: add ch_hotplug.{h,c} files to CH build
>   ch: call into ch_hotplug implementation
>   ch: pass disk alias to CHV
>   ch: add ch_alias.{c,h} for device alias handling
>   ch: add virCHMonitorPut function
>   ch: use MonitorPut in MonitorPutNoContent
>   ch: add disk attach functionality
>   ch: assign aliases in ProcessPrepareDomain
>   ch: Add virCHMonitorRemoveDevice function
>   ch: implement disk device hot removal
>   NEWS: announce disk hotplug support for ch

I didn't have time (or expertise on the cloud hypervisor specifics) to
do proper review but I've noticed that the patches introduce temporary
problems (e.g. failed build) in between the patches.

Our guidelines state that both the build and tests must pass after each
commit (to e.g. allow good bisectability):

 https://libvirt.org/hacking.html#preparing-patches

Make sure that you run a build and test after each commit. No need to
send another series yet, but be prepared to fix those problems.

Some things my testing showed:

../../../libvirt/src/ch/ch_driver.c:2349:46: error: omitting the parameter name in a function definition is a C23 extension [-Werror,-Wc23-extensions]
 2349 |                           const char */*xml*/,
      |                                              ^
1 error generated.



- addition to po/POTFILES being misplaced
- misplaced function declarations in header files (broken build where
  function is not declared yet)