[PATCH 0/6] pc: support user provided NIC naming/indexing

Igor Mammedov posted 6 patches 3 years ago
Test checkpatch failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210315180102.3008391-1-imammedo@redhat.com
Maintainers: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Aurelien Jarno <aurelien@aurel32.net>, Richard Henderson <richard.henderson@linaro.org>
include/hw/acpi/aml-build.h      |   3 +
include/hw/acpi/pci.h            |   1 +
include/hw/acpi/pcihp.h          |   9 ++-
include/hw/pci/pci.h             |   1 +
hw/acpi/aml-build.c              |  28 ++++++++
hw/acpi/pci.c                    |   1 -
hw/acpi/pcihp.c                  | 104 ++++++++++++++++++++++++++-
hw/acpi/piix4.c                  |   3 +-
hw/acpi/trace-events             |   2 +
hw/i386/acpi-build.c             | 118 +++++++++++++++++++++++++++++--
hw/pci/pci.c                     |   1 +
tests/data/acpi/pc/DSDT          | Bin 5065 -> 6002 bytes
tests/data/acpi/pc/DSDT.acpihmat | Bin 6390 -> 7327 bytes
tests/data/acpi/pc/DSDT.bridge   | Bin 6924 -> 8668 bytes
tests/data/acpi/pc/DSDT.cphp     | Bin 5529 -> 6466 bytes
tests/data/acpi/pc/DSDT.dimmpxm  | Bin 6719 -> 7656 bytes
tests/data/acpi/pc/DSDT.hpbridge | Bin 5026 -> 5969 bytes
tests/data/acpi/pc/DSDT.ipmikcs  | Bin 5137 -> 6074 bytes
tests/data/acpi/pc/DSDT.memhp    | Bin 6424 -> 7361 bytes
tests/data/acpi/pc/DSDT.nohpet   | Bin 4923 -> 5860 bytes
tests/data/acpi/pc/DSDT.numamem  | Bin 5071 -> 6008 bytes
tests/data/acpi/pc/DSDT.roothp   | Bin 5261 -> 6210 bytes
22 files changed, 261 insertions(+), 10 deletions(-)
[PATCH 0/6] pc: support user provided NIC naming/indexing
Posted by Igor Mammedov 3 years ago
Changes since rfc:
  * make sure that acpi-index unique
  * use dedicated MMIO register for it
  * drop syntetic acpi-label field in _DSM
  * add more comments

Series implements support for 'onboard' naming scheme for network
interfaces (1), which is based on PCI firmware spec and lets user
to explicitly specify index that will be used by guest to name
network interface, ex:
    -device e1000,acpi-index=33
should make guest rename NIC name to 'eno33' where 'eno' is default
prefix for this scheme.

Hope is that it will allow to simplify launching VMs from
template disk images with different set VM configurations
without need to reconfigure guest network intrfaces or
risk of loosing network connectivity.

For more detailed description/examples see patches [3-4/5]

1)
 https://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/ 

Git repo for testing:
   https://github.com/imammedo/qemu/branches acpi-index-v1


CC: laine@redhat.com
CC: mst@redhat.com
CC: jusual@redhat.com

Igor Mammedov (6):
  tests: acpi: temporary whitelist DSDT changes
  pci: introduce apci-index property for PCI device
  pci: acpi: ensure that acpi-index is unique
  acpi: add aml_to_decimalstring() and aml_call6() helpers
  pci: acpi: add _DSM method to PCI devices
  tests: acpi: update expected blobs

 include/hw/acpi/aml-build.h      |   3 +
 include/hw/acpi/pci.h            |   1 +
 include/hw/acpi/pcihp.h          |   9 ++-
 include/hw/pci/pci.h             |   1 +
 hw/acpi/aml-build.c              |  28 ++++++++
 hw/acpi/pci.c                    |   1 -
 hw/acpi/pcihp.c                  | 104 ++++++++++++++++++++++++++-
 hw/acpi/piix4.c                  |   3 +-
 hw/acpi/trace-events             |   2 +
 hw/i386/acpi-build.c             | 118 +++++++++++++++++++++++++++++--
 hw/pci/pci.c                     |   1 +
 tests/data/acpi/pc/DSDT          | Bin 5065 -> 6002 bytes
 tests/data/acpi/pc/DSDT.acpihmat | Bin 6390 -> 7327 bytes
 tests/data/acpi/pc/DSDT.bridge   | Bin 6924 -> 8668 bytes
 tests/data/acpi/pc/DSDT.cphp     | Bin 5529 -> 6466 bytes
 tests/data/acpi/pc/DSDT.dimmpxm  | Bin 6719 -> 7656 bytes
 tests/data/acpi/pc/DSDT.hpbridge | Bin 5026 -> 5969 bytes
 tests/data/acpi/pc/DSDT.ipmikcs  | Bin 5137 -> 6074 bytes
 tests/data/acpi/pc/DSDT.memhp    | Bin 6424 -> 7361 bytes
 tests/data/acpi/pc/DSDT.nohpet   | Bin 4923 -> 5860 bytes
 tests/data/acpi/pc/DSDT.numamem  | Bin 5071 -> 6008 bytes
 tests/data/acpi/pc/DSDT.roothp   | Bin 5261 -> 6210 bytes
 22 files changed, 261 insertions(+), 10 deletions(-)

-- 
2.27.0


Re: [PATCH 0/6] pc: support user provided NIC naming/indexing
Posted by no-reply@patchew.org 3 years ago
Patchew URL: https://patchew.org/QEMU/20210315180102.3008391-1-imammedo@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210315180102.3008391-1-imammedo@redhat.com
Subject: [PATCH 0/6] pc: support user provided NIC naming/indexing

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20210315180102.3008391-1-imammedo@redhat.com -> patchew/20210315180102.3008391-1-imammedo@redhat.com
Switched to a new branch 'test'
c0692b4 tests: acpi: update expected blobs
1a843d9 pci: acpi: add _DSM method to PCI devices
6b55586 acpi: add aml_to_decimalstring() and aml_call6() helpers
7b887f4 pci: acpi: ensure that acpi-index is unique
4056291 pci: introduce apci-index property for PCI device
9bd3a1b tests: acpi: temporary whitelist DSDT changes

=== OUTPUT BEGIN ===
1/6 Checking commit 9bd3a1b94c0f (tests: acpi: temporary whitelist DSDT changes)
2/6 Checking commit 40562915acde (pci: introduce apci-index property for PCI device)
ERROR: Macros with complex values should be enclosed in parenthesis
#284: FILE: include/hw/acpi/pcihp.h:77:
+#define VMSTATE_PCI_HOTPLUG(pcihp, state, test_pcihp, test_acpi_index) \
         VMSTATE_UINT32_TEST(pcihp.hotplug_select, state, \
                             test_pcihp), \
         VMSTATE_STRUCT_ARRAY_TEST(pcihp.acpi_pcihp_pci_status, state, \
                                   ACPI_PCIHP_MAX_HOTPLUG_BUS, \
                                   test_pcihp, 1, \
                                   vmstate_acpi_pcihp_pci_status, \
-                                  AcpiPciHpPciStatus)
+                                  AcpiPciHpPciStatus), \
+        VMSTATE_UINT32_TEST(pcihp.acpi_index, state, \
+                            test_acpi_index)

total: 1 errors, 0 warnings, 197 lines checked

Patch 2/6 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/6 Checking commit 7b887f42419a (pci: acpi: ensure that acpi-index is unique)
4/6 Checking commit 6b555869ad66 (acpi: add aml_to_decimalstring() and aml_call6() helpers)
WARNING: Block comments use a leading /* on a separate line
#27: FILE: hw/acpi/aml-build.c:640:
+    Aml *var = aml_opcode(0x97 /* ToDecimalStringOp */);

WARNING: Block comments use a leading /* on a separate line
#32: FILE: hw/acpi/aml-build.c:645:
+        build_append_byte(var->buf, 0x00 /* NullNameOp */);

total: 0 errors, 2 warnings, 55 lines checked

Patch 4/6 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
5/6 Checking commit 1a843d9de55e (pci: acpi: add _DSM method to PCI devices)
WARNING: Block comments use a leading /* on a separate line
#127: FILE: hw/i386/acpi-build.c:568:
+                        1 /* have supported functions */ |

total: 0 errors, 1 warnings, 140 lines checked

Patch 5/6 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/6 Checking commit c0692b4b7df0 (tests: acpi: update expected blobs)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210315180102.3008391-1-imammedo@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [PATCH 0/6] pc: support user provided NIC naming/indexing
Posted by Igor Mammedov 3 years ago
On Mon, 15 Mar 2021 11:36:49 -0700 (PDT)
no-reply@patchew.org wrote:

> Patchew URL: https://patchew.org/QEMU/20210315180102.3008391-1-imammedo@redhat.com/
> 
> 
> 
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 
> Type: series
> Message-id: 20210315180102.3008391-1-imammedo@redhat.com
> Subject: [PATCH 0/6] pc: support user provided NIC naming/indexing
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> git rev-parse base > /dev/null || exit 0
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> git config --local diff.algorithm histogram
> ./scripts/checkpatch.pl --mailback base..
> === TEST SCRIPT END ===
> 
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> From https://github.com/patchew-project/qemu
>  * [new tag]         patchew/20210315180102.3008391-1-imammedo@redhat.com -> patchew/20210315180102.3008391-1-imammedo@redhat.com
> Switched to a new branch 'test'
> c0692b4 tests: acpi: update expected blobs
> 1a843d9 pci: acpi: add _DSM method to PCI devices
> 6b55586 acpi: add aml_to_decimalstring() and aml_call6() helpers
> 7b887f4 pci: acpi: ensure that acpi-index is unique
> 4056291 pci: introduce apci-index property for PCI device
> 9bd3a1b tests: acpi: temporary whitelist DSDT changes
> 
> === OUTPUT BEGIN ===
> 1/6 Checking commit 9bd3a1b94c0f (tests: acpi: temporary whitelist DSDT changes)
> 2/6 Checking commit 40562915acde (pci: introduce apci-index property for PCI device)
> ERROR: Macros with complex values should be enclosed in parenthesis
> #284: FILE: include/hw/acpi/pcihp.h:77:
> +#define VMSTATE_PCI_HOTPLUG(pcihp, state, test_pcihp, test_acpi_index) \
>          VMSTATE_UINT32_TEST(pcihp.hotplug_select, state, \
>                              test_pcihp), \
>          VMSTATE_STRUCT_ARRAY_TEST(pcihp.acpi_pcihp_pci_status, state, \
>                                    ACPI_PCIHP_MAX_HOTPLUG_BUS, \
>                                    test_pcihp, 1, \
>                                    vmstate_acpi_pcihp_pci_status, \
> -                                  AcpiPciHpPciStatus)
> +                                  AcpiPciHpPciStatus), \
> +        VMSTATE_UINT32_TEST(pcihp.acpi_index, state, \
> +                            test_acpi_index)
probably false positive
(I don't know what's wrong here and how to fix it)

> total: 1 errors, 0 warnings, 197 lines checked
> 
> Patch 2/6 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.

below warnings are intentional to follow comments style we use within aml-build.c

> 3/6 Checking commit 7b887f42419a (pci: acpi: ensure that acpi-index is unique)
> 4/6 Checking commit 6b555869ad66 (acpi: add aml_to_decimalstring() and aml_call6() helpers)
> WARNING: Block comments use a leading /* on a separate line
> #27: FILE: hw/acpi/aml-build.c:640:
> +    Aml *var = aml_opcode(0x97 /* ToDecimalStringOp */);
> 
> WARNING: Block comments use a leading /* on a separate line
> #32: FILE: hw/acpi/aml-build.c:645:
> +        build_append_byte(var->buf, 0x00 /* NullNameOp */);
> 
> total: 0 errors, 2 warnings, 55 lines checked
> 
> Patch 4/6 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 5/6 Checking commit 1a843d9de55e (pci: acpi: add _DSM method to PCI devices)
> WARNING: Block comments use a leading /* on a separate line
> #127: FILE: hw/i386/acpi-build.c:568:
> +                        1 /* have supported functions */ |
> 
> total: 0 errors, 1 warnings, 140 lines checked
> 
> Patch 5/6 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 6/6 Checking commit c0692b4b7df0 (tests: acpi: update expected blobs)
> === OUTPUT END ===
> 
> Test command exited with code: 1
> 
> 
> The full log is available at
> http://patchew.org/logs/20210315180102.3008391-1-imammedo@redhat.com/testing.checkpatch/?type=message.
> ---
> Email generated automatically by Patchew [https://patchew.org/].
> Please send your feedback to patchew-devel@redhat.com


Re: [PATCH 0/6] pc: support user provided NIC naming/indexing
Posted by Michael S. Tsirkin 2 years, 10 months ago
On Mon, Mar 15, 2021 at 02:00:56PM -0400, Igor Mammedov wrote:
> 
> Changes since rfc:
>   * make sure that acpi-index unique
>   * use dedicated MMIO register for it
>   * drop syntetic acpi-label field in _DSM
>   * add more comments


Could you rebase and repost please?
Some comments in commit log about use for non-NIC devices
would be helpful (even if it's just that "currently does not seem to be useful for
non-NICs").

> Series implements support for 'onboard' naming scheme for network
> interfaces (1), which is based on PCI firmware spec and lets user
> to explicitly specify index that will be used by guest to name
> network interface, ex:
>     -device e1000,acpi-index=33
> should make guest rename NIC name to 'eno33' where 'eno' is default
> prefix for this scheme.
> 
> Hope is that it will allow to simplify launching VMs from
> template disk images with different set VM configurations
> without need to reconfigure guest network intrfaces or
> risk of loosing network connectivity.
> 
> For more detailed description/examples see patches [3-4/5]
> 
> 1)
>  https://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/ 
> 
> Git repo for testing:
>    https://github.com/imammedo/qemu/branches acpi-index-v1
> 
> 
> CC: laine@redhat.com
> CC: mst@redhat.com
> CC: jusual@redhat.com
> 
> Igor Mammedov (6):
>   tests: acpi: temporary whitelist DSDT changes
>   pci: introduce apci-index property for PCI device
>   pci: acpi: ensure that acpi-index is unique
>   acpi: add aml_to_decimalstring() and aml_call6() helpers
>   pci: acpi: add _DSM method to PCI devices
>   tests: acpi: update expected blobs
> 
>  include/hw/acpi/aml-build.h      |   3 +
>  include/hw/acpi/pci.h            |   1 +
>  include/hw/acpi/pcihp.h          |   9 ++-
>  include/hw/pci/pci.h             |   1 +
>  hw/acpi/aml-build.c              |  28 ++++++++
>  hw/acpi/pci.c                    |   1 -
>  hw/acpi/pcihp.c                  | 104 ++++++++++++++++++++++++++-
>  hw/acpi/piix4.c                  |   3 +-
>  hw/acpi/trace-events             |   2 +
>  hw/i386/acpi-build.c             | 118 +++++++++++++++++++++++++++++--
>  hw/pci/pci.c                     |   1 +
>  tests/data/acpi/pc/DSDT          | Bin 5065 -> 6002 bytes
>  tests/data/acpi/pc/DSDT.acpihmat | Bin 6390 -> 7327 bytes
>  tests/data/acpi/pc/DSDT.bridge   | Bin 6924 -> 8668 bytes
>  tests/data/acpi/pc/DSDT.cphp     | Bin 5529 -> 6466 bytes
>  tests/data/acpi/pc/DSDT.dimmpxm  | Bin 6719 -> 7656 bytes
>  tests/data/acpi/pc/DSDT.hpbridge | Bin 5026 -> 5969 bytes
>  tests/data/acpi/pc/DSDT.ipmikcs  | Bin 5137 -> 6074 bytes
>  tests/data/acpi/pc/DSDT.memhp    | Bin 6424 -> 7361 bytes
>  tests/data/acpi/pc/DSDT.nohpet   | Bin 4923 -> 5860 bytes
>  tests/data/acpi/pc/DSDT.numamem  | Bin 5071 -> 6008 bytes
>  tests/data/acpi/pc/DSDT.roothp   | Bin 5261 -> 6210 bytes
>  22 files changed, 261 insertions(+), 10 deletions(-)
> 
> -- 
> 2.27.0