[pve-devel] [PATCH container 1/2] cloud-init: add support for multiple IP addresses

Wolfgang Bumiller w.bumiller at proxmox.com
Wed Oct 24 09:45:39 CEST 2018


Thanks for diving into the code, but please consider the following:

A big part of these patches seem to be reformatting code, which makes
it hard to find the actual functionality changes.
Please do not introduce functionality changes and coding style changes
in a single patch. Many of the alignment/indentation changes also seem
to not fit into our usual code base[1], so please avoid that.

If you find actual inconsistencies in coding style in a file (of which
we certainly have accumulated a bunch), please first fix that as a
separate patch on its own, and then add your desired functionality
changes in a another patch on top of it, this makes it a lot easier to
review. (Otherwise introduce only the functionality changes while
sticking to whatever style is being used in the section you're
changing.)

As for the patch submission itself: We try to put the repository name
into the [PATCH] (somewhat shortened usually, in your case:
[PATCH common] and [PATCH qemu-server]).
If you're unsure about tags - [PATCH] on its own works if it's clear, or
mentioned in a cover letter, which repository the patches go into.

In the 'cloud-init: ...' patches I'd also like to see a description of
how this affects the VM config in the commit message.

Finally, the cover letter should also describe how exactly you want
multiple addresses to be provided, because this kind of change is
something we'd very much like to try to keep in sync with how we manage
networking for containers (despite cloud-init's ... wild and ... untamed
way of ... "working").

[1] https://pve.proxmox.com/wiki/Perl_Style_Guide


On Wed, Oct 24, 2018 at 08:57:21AM +0200, Julian Pawlowski wrote:
> This will prepare the JSON schema to allow using a key multiple times.
> It will be needed for cloud-init to configure multiple IP addresses to a
> single network interface.
> 
> Julian Pawlowski (2):
>   qemu-server: add support for multiple IP addresses
>   qemu-server: fix default_key check for multi-key support
> 
>  src/PVE/JSONSchema.pm | 97 ++++++++++++++++++++++++++-----------------
>  1 file changed, 60 insertions(+), 37 deletions(-)
> 
> --
> 2.19.1




More information about the pve-devel mailing list