Re: [Qemu-devel] [libvirt] [PATCH for-4.0 v4 0/2] virtio: Provide version-specific variants of virtio PCI devices
On Wed, Mar 06, 2019 at 09:30:32AM +0100, Ján Tomko wrote: > On Wed, Mar 06, 2019 at 08:41:48AM +0100, Peter Krempa wrote: > > On Tue, Mar 05, 2019 at 16:56:43 +0100, Andrea Bolognani wrote: > > > On Tue, 2019-03-05 at 15:38 +0100, Gerd Hoffmann wrote: > > > > [...] > > > > > So I agree neither scenario is exactly perfect, but I still think > > > adding non-transitional alias devices would overall be more > > > user-friendly. > > > > I don't think it makes sense to add it at the qemu level. From libvirt's > > point of view users should be shielded from any qemu impl detail or > > inconsistency as libvirt is the 'user friendly'[1] layer. In qemu the > > devices would be the same and thus does not make sense to do that > > because it would be more confusing. > > > > You can argue that we should add the alias at the libvirt level though. > > > > You can, but please don't. Indeed, at the libvirt level we've always tried to take the view that there should be one way to expressing each concept/feature. Adding new names / xml elements that duplicate existing supported concepts to make things "consistent" is a slippery slope becasue there are 100's of places to which that can apply when you have hindsight. It is not going to make a significant difference to how "user friendly" libvirt is - that is not a core goal in its own right at the API / XML schema level. It is can be a factor in deciding between multiple competing designs when first adding a feature, but it isn't a reason to add duplication in the API / XML. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [libvirt] [PATCH for-4.0 v4 0/2] virtio: Provide version-specific variants of virtio PCI devices
On Wed, 2019-03-06 at 09:30 +0100, Ján Tomko wrote: > On Wed, Mar 06, 2019 at 08:41:48AM +0100, Peter Krempa wrote: > > On Tue, Mar 05, 2019 at 16:56:43 +0100, Andrea Bolognani wrote: > > > So I agree neither scenario is exactly perfect, but I still think > > > adding non-transitional alias devices would overall be more > > > user-friendly. > > > > I don't think it makes sense to add it at the qemu level. From libvirt's > > point of view users should be shielded from any qemu impl detail or > > inconsistency as libvirt is the 'user friendly'[1] layer. In qemu the > > devices would be the same and thus does not make sense to do that > > because it would be more confusing. > > > > You can argue that we should add the alias at the libvirt level though. > > You can, but please don't. It would seem nobody except me thinks this is a good idea, so I guess I'll just drop it now. -- Andrea Bolognani / Red Hat / Virtualization
Re: [Qemu-devel] [libvirt] [PATCH for-4.0 v4 0/2] virtio: Provide version-specific variants of virtio PCI devices
On Wed, Mar 06, 2019 at 08:41:48AM +0100, Peter Krempa wrote: On Tue, Mar 05, 2019 at 16:56:43 +0100, Andrea Bolognani wrote: On Tue, 2019-03-05 at 15:38 +0100, Gerd Hoffmann wrote: [...] So I agree neither scenario is exactly perfect, but I still think adding non-transitional alias devices would overall be more user-friendly. I don't think it makes sense to add it at the qemu level. From libvirt's point of view users should be shielded from any qemu impl detail or inconsistency as libvirt is the 'user friendly'[1] layer. In qemu the devices would be the same and thus does not make sense to do that because it would be more confusing. You can argue that we should add the alias at the libvirt level though. You can, but please don't. Jano [1] Yes. I'm aware that statement is quite ironical. -- libvir-list mailing list libvir-l...@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: PGP signature
Re: [Qemu-devel] [libvirt] [PATCH for-4.0 v4 0/2] virtio: Provide version-specific variants of virtio PCI devices
On Tue, Mar 05, 2019 at 16:56:43 +0100, Andrea Bolognani wrote: > On Tue, 2019-03-05 at 15:38 +0100, Gerd Hoffmann wrote: [...] > So I agree neither scenario is exactly perfect, but I still think > adding non-transitional alias devices would overall be more > user-friendly. I don't think it makes sense to add it at the qemu level. From libvirt's point of view users should be shielded from any qemu impl detail or inconsistency as libvirt is the 'user friendly'[1] layer. In qemu the devices would be the same and thus does not make sense to do that because it would be more confusing. You can argue that we should add the alias at the libvirt level though. [1] Yes. I'm aware that statement is quite ironical. signature.asc Description: PGP signature
Re: [Qemu-devel] [libvirt] [PATCH for-4.0 v4 0/2] virtio: Provide version-specific variants of virtio PCI devices
Patchew URL:
https://patchew.org/QEMU/20181205195704.17605-1-ehabk...@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Message-id: 20181205195704.17605-1-ehabk...@redhat.com
Subject: [libvirt] [PATCH for-4.0 v4 0/2] virtio: Provide version-specific
variants of virtio PCI devices
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
486a758 virtio: Provide version-specific variants of virtio PCI devices
85361d9 virtio: Helper for registering virtio device types
=== OUTPUT BEGIN ===
Checking PATCH 1/2: virtio: Helper for registering virtio device types...
WARNING: line over 80 characters
#496: FILE: hw/virtio/virtio-pci.h:443:
+ * Implements both INTERFACE_PCIE_DEVICE and
INTERFACE_CONVENTIONAL_PCI_DEVICE,
WARNING: line over 80 characters
#505: FILE: hw/virtio/virtio-pci.h:452:
+ * Implements both INTERFACE_PCIE_DEVICE and
INTERFACE_CONVENTIONAL_PCI_DEVICE.
total: 0 errors, 2 warnings, 469 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/2: virtio: Provide version-specific variants of virtio PCI
devices...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#316:
new file mode 100644
ERROR: line over 90 characters
#372: FILE: tests/acceptance/virtio_version.py:52:
+return devtype in [d['name'] for d in vm.command('qom-list-types',
implements=implements)]
WARNING: line over 80 characters
#427: FILE: tests/acceptance/virtio_version.py:107:
+dev_1_0, nt_ifaces = self.run_device('%s-non-transitional' %
(qemu_devtype))
WARNING: line over 80 characters
#451: FILE: tests/acceptance/virtio_version.py:131:
+dev_trans, trans_ifaces = self.run_device('%s-transitional' %
(qemu_devtype))
total: 1 errors, 3 warnings, 404 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20181205195704.17605-1-ehabk...@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com
