Re: [pve-devel] [PATCH storage 1/9] copy OVF.pm from qemu-server

2024-04-17 Thread Thomas Lamprecht 
Am 17/04/2024 um 11:19 schrieb Fiona Ebner:
> Am 16.04.24 um 17:02 schrieb Thomas Lamprecht:
>> high-level nit: this, and most of the ESXi one, should go into another module
>> name space, e.g. PVE::GuestImport:: (or if that's to long, or we really are 
>> sure
>> that other stuff can be imported (I doubt it), then just PVE::Import might be
>> fine too).
>>
> 
> Hmm, ESXiPlugin.pm is a storage plugin, so it does fit. But no

Yes, ESXiPlugin _is_ a storage plugin, and it must stay there, but about
80% of it's code is not related to being a storage plugin but for importing
only, parts of it might be even shareable with other such import related
stuff. So what I meant with "**most** of the ESXi one" is that I'd separate
these parts from the storage plugin specific code, not moving it completely.

> objections to moving it from my side either. And fully agree that OVF.pm
> should live somewhere else, it is not a storage plugin.



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH storage 1/9] copy OVF.pm from qemu-server

2024-04-17 Thread Fiona Ebner 
Am 16.04.24 um 17:02 schrieb Thomas Lamprecht:
> Am 16/04/2024 um 15:18 schrieb Dominik Csapak:
>> copies the OVF.pm and relevant ovf tests from qemu-server.
>> We need it here, and it uses PVE::Storage already, and since there is no
>> intermediary package/repository we could put it, it seems fitting in
>> here.
>>
>> Signed-off-by: Dominik Csapak 

Except for the location of the module:

Reviewed-by: Fiona Ebner 
> 
> high-level nit: this, and most of the ESXi one, should go into another module
> name space, e.g. PVE::GuestImport:: (or if that's to long, or we really are 
> sure
> that other stuff can be imported (I doubt it), then just PVE::Import might be
> fine too).
> 

Hmm, ESXiPlugin.pm is a storage plugin, so it does fit. But no
objections to moving it from my side either. And fully agree that OVF.pm
should live somewhere else, it is not a storage plugin.


___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH storage 1/9] copy OVF.pm from qemu-server

2024-04-16 Thread Thomas Lamprecht 
Am 16/04/2024 um 15:18 schrieb Dominik Csapak:
> copies the OVF.pm and relevant ovf tests from qemu-server.
> We need it here, and it uses PVE::Storage already, and since there is no
> intermediary package/repository we could put it, it seems fitting in
> here.
> 
> Signed-off-by: Dominik Csapak 
> ---
>  src/PVE/Storage/Makefile  |   1 +
>  src/PVE/Storage/OVF.pm| 242 ++
>  src/test/Makefile |   5 +-
>  src/test/ovf_manifests/Win10-Liz-disk1.vmdk   | Bin 0 -> 65536 bytes
>  src/test/ovf_manifests/Win10-Liz.ovf  | 142 ++
>  .../ovf_manifests/Win10-Liz_no_default_ns.ovf | 142 ++
>  .../ovf_manifests/Win_2008_R2_two-disks.ovf   | 145 +++
>  src/test/ovf_manifests/disk1.vmdk | Bin 0 -> 65536 bytes
>  src/test/ovf_manifests/disk2.vmdk | Bin 0 -> 65536 bytes
>  src/test/run_ovf_tests.pl |  71 +
>  10 files changed, 747 insertions(+), 1 deletion(-)
>  create mode 100644 src/PVE/Storage/OVF.pm
>  create mode 100644 src/test/ovf_manifests/Win10-Liz-disk1.vmdk
>  create mode 100755 src/test/ovf_manifests/Win10-Liz.ovf
>  create mode 100755 src/test/ovf_manifests/Win10-Liz_no_default_ns.ovf
>  create mode 100755 src/test/ovf_manifests/Win_2008_R2_two-disks.ovf
>  create mode 100644 src/test/ovf_manifests/disk1.vmdk
>  create mode 100644 src/test/ovf_manifests/disk2.vmdk
>  create mode 100755 src/test/run_ovf_tests.pl
> 
> diff --git a/src/PVE/Storage/Makefile b/src/PVE/Storage/Makefile
> index d5cc942..2daa0da 100644
> --- a/src/PVE/Storage/Makefile
> +++ b/src/PVE/Storage/Makefile
> @@ -14,6 +14,7 @@ SOURCES= \
>   PBSPlugin.pm \
>   BTRFSPlugin.pm \
>   LvmThinPlugin.pm \
> + OVF.pm \
>   ESXiPlugin.pm
>  
>  .PHONY: install
> diff --git a/src/PVE/Storage/OVF.pm b/src/PVE/Storage/OVF.pm
> new file mode 100644
> index 000..90ca453
> --- /dev/null
> +++ b/src/PVE/Storage/OVF.pm
> @@ -0,0 +1,242 @@
> +# Open Virtualization Format import routines
> +# https://www.dmtf.org/standards/ovf
> +package PVE::Storage::OVF;
> +


high-level nit: this, and most of the ESXi one, should go into another module
name space, e.g. PVE::GuestImport:: (or if that's to long, or we really are sure
that other stuff can be imported (I doubt it), then just PVE::Import might be
fine too).


___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel