Re: [Qemu-devel] [PATCH 0/1] BZ#1733165: network-get-interfaces Chinese NIC name
PING On Mon, Aug 19, 2019 at 4:22 PM Bishara AbuHattoum wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1733165 > > Uppon renaming a NIC to a Chinese name and invoking the network get > interfaces command, guest-network-get-interfaces, the returned name > field has the (\ufffd) value for each Chinese character the NIC name > had, this value is the indication that the code page does not have the > decoding information for the given character. > > The suggested fix is to use the CP_UTF8 code page for decoding the NIC's > name instead of the CP_ACP code page. > > Bishara AbuHattoum (1): > qga-win: network-get-interfaces command name field bug fix > > qga/commands-win32.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > -- > 2.17.2 > > >
Re: [Qemu-devel] [PATCH 0/1] Fix cacheline detection on FreeBSD/powerpc
On Wed, 21 Aug 2019 10:25:45 +0200 Laurent Vivier wrote: > This is the patch originally sent by Justin, modified > to change the parameter size on FreeBSD only. > > Justin, could you review and test on FreeBSD? > Peter, could you run "make check" on your MacOS X host? > > Thanks, > Laurent > > Justin Hibbits (1): > Fix cacheline detection on FreeBSD/powerpc. > > util/cacheinfo.c | 23 +-- > 1 file changed, 13 insertions(+), 10 deletions(-) > Hi Laurent, I applied the patch against the FreeBSD emulators/qemu-devel (qemu 3.1.0) port, and it was successful. All good on this end! - Justin
Re: [Qemu-devel] [PATCH 0/1] Fix cacheline detection on FreeBSD/powerpc
On Wed, 21 Aug 2019 10:25:45 +0200 Laurent Vivier wrote: > This is the patch originally sent by Justin, modified > to change the parameter size on FreeBSD only. > > Justin, could you review and test on FreeBSD? > Peter, could you run "make check" on your MacOS X host? > > Thanks, > Laurent > > Justin Hibbits (1): > Fix cacheline detection on FreeBSD/powerpc. > > util/cacheinfo.c | 23 +-- > 1 file changed, 13 insertions(+), 10 deletions(-) > Hi, Sorry for the delay, I'll get to testing it tonight or tomorrow. The patch looks good from inspection. - Justin
[Qemu-devel] [PATCH 0/1] Fix LP#1841442 for powerpc
I only attempt to fix this bug for powerpc at the moment. For x86_64... there is no existing attempt to handle fp exceptions at all. It will be quite the job to fix. r~ Richard Henderson (1): target/ppc: Fix do_float_check_status vs inexact target/ppc/fpu_helper.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) -- 2.17.1
[Qemu-devel] [PATCH 0/1] NBD protocol change to add fast zero support
See the cross-post cover letter for details: https://www.redhat.com/archives/libguestfs/2019-August/msg00322.html Eric Blake (1): protocol: Add NBD_CMD_FLAG_FAST_ZERO doc/proto.md | 50 +- 1 file changed, 49 insertions(+), 1 deletion(-) -- 2.21.0
[Qemu-devel] [PATCH 0/1] Fix cacheline detection on FreeBSD/powerpc
This is the patch originally sent by Justin, modified to change the parameter size on FreeBSD only. Justin, could you review and test on FreeBSD? Peter, could you run "make check" on your MacOS X host? Thanks, Laurent Justin Hibbits (1): Fix cacheline detection on FreeBSD/powerpc. util/cacheinfo.c | 23 +-- 1 file changed, 13 insertions(+), 10 deletions(-) -- 2.21.0
[Qemu-devel] [PATCH 0/1] BZ#1733165: network-get-interfaces Chinese NIC name
https://bugzilla.redhat.com/show_bug.cgi?id=1733165 Uppon renaming a NIC to a Chinese name and invoking the network get interfaces command, guest-network-get-interfaces, the returned name field has the (\ufffd) value for each Chinese character the NIC name had, this value is the indication that the code page does not have the decoding information for the given character. The suggested fix is to use the CP_UTF8 code page for decoding the NIC's name instead of the CP_ACP code page. Bishara AbuHattoum (1): qga-win: network-get-interfaces command name field bug fix qga/commands-win32.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) -- 2.17.2
Re: [Qemu-devel] [PATCH 0/1] display/bochs: fix pcie support (qemu security issue)
On Mon, 12 Aug 2019 at 16:48, Alex Williamson wrote: > > On Mon, 12 Aug 2019 16:38:05 +0100 > Peter Maydell wrote: > > > On Mon, 12 Aug 2019 at 16:35, Alex Williamson > > wrote: > > > Quoting new commit log: > > > > > > This makes sure the pci config space allocation is big enough, > > > so accessing the PCIe extended config space doesn't overflow > > > the pci config space buffer. > > > > > > PCI(e) config space is guest writable. Writes are limited > > > bywrite mask (which probably is also filled with random stuff), > > > so the guest can only flip enabled bits. But I suspect it > > > still might be exploitable, so rather serious because it might > > > be a host escape for the guest. On the other hand the device > > > is probably not yet in widespread use. > > > > > > Mitigation: use "-device bochs-display" as conventional pci > > > device only. > > > > > > Is it clear to others that this mitigation remark seems to be > > > referencing an alternative configuration constraint to avoid the issue > > > rather than what's actually implemented in this patch? IOW, if we > > > never place the bochs-display device into a PCIe hierarchy, then > > > extended config space is never accessible to the guest anyway, and > > > there is no issue. I think this was meant to be an alternative to the > > > patch but the enforcement of that would happen above QEMU, probably why > > > it was mentioned in the cover letter rather than the original commit > > > log. Thanks, > > > > Yeah, that's unclear in retrospect. How about: > > > > # (For a QEMU version without this commit, a mitigation for the > > # bug is available: use "-device bochs-display" as a conventional pci > > # device only.) > > Yes, better. Thanks, Cool. Updated commit message now pushed to master. -- PMM
Re: [Qemu-devel] [PATCH 0/1] display/bochs: fix pcie support (qemu security issue)
On Mon, 12 Aug 2019 16:38:05 +0100 Peter Maydell wrote: > On Mon, 12 Aug 2019 at 16:35, Alex Williamson > wrote: > > Quoting new commit log: > > > > This makes sure the pci config space allocation is big enough, > > so accessing the PCIe extended config space doesn't overflow > > the pci config space buffer. > > > > PCI(e) config space is guest writable. Writes are limited > > bywrite mask (which probably is also filled with random stuff), > > so the guest can only flip enabled bits. But I suspect it > > still might be exploitable, so rather serious because it might > > be a host escape for the guest. On the other hand the device > > is probably not yet in widespread use. > > > > Mitigation: use "-device bochs-display" as conventional pci > > device only. > > > > Is it clear to others that this mitigation remark seems to be > > referencing an alternative configuration constraint to avoid the issue > > rather than what's actually implemented in this patch? IOW, if we > > never place the bochs-display device into a PCIe hierarchy, then > > extended config space is never accessible to the guest anyway, and > > there is no issue. I think this was meant to be an alternative to the > > patch but the enforcement of that would happen above QEMU, probably why > > it was mentioned in the cover letter rather than the original commit > > log. Thanks, > > Yeah, that's unclear in retrospect. How about: > > # (For a QEMU version without this commit, a mitigation for the > # bug is available: use "-device bochs-display" as a conventional pci > # device only.) Yes, better. Thanks, Alex
Re: [Qemu-devel] [PATCH 0/1] display/bochs: fix pcie support (qemu security issue)
On Mon, 12 Aug 2019 at 16:35, Alex Williamson wrote: > Quoting new commit log: > > This makes sure the pci config space allocation is big enough, > so accessing the PCIe extended config space doesn't overflow > the pci config space buffer. > > PCI(e) config space is guest writable. Writes are limited > bywrite mask (which probably is also filled with random stuff), > so the guest can only flip enabled bits. But I suspect it > still might be exploitable, so rather serious because it might > be a host escape for the guest. On the other hand the device > is probably not yet in widespread use. > > Mitigation: use "-device bochs-display" as conventional pci > device only. > > Is it clear to others that this mitigation remark seems to be > referencing an alternative configuration constraint to avoid the issue > rather than what's actually implemented in this patch? IOW, if we > never place the bochs-display device into a PCIe hierarchy, then > extended config space is never accessible to the guest anyway, and > there is no issue. I think this was meant to be an alternative to the > patch but the enforcement of that would happen above QEMU, probably why > it was mentioned in the cover letter rather than the original commit > log. Thanks, Yeah, that's unclear in retrospect. How about: # (For a QEMU version without this commit, a mitigation for the # bug is available: use "-device bochs-display" as a conventional pci # device only.) ? thanks -- PMM
Re: [Qemu-devel] [PATCH 0/1] display/bochs: fix pcie support (qemu security issue)
On Mon, 12 Aug 2019 14:39:53 +0100 Peter Maydell wrote: > On Mon, 12 Aug 2019 at 13:51, Philippe Mathieu-Daudé > wrote: > > > > On 8/12/19 2:45 PM, Paolo Bonzini wrote: > > > On 12/08/19 08:52, Gerd Hoffmann wrote: > > >> Just found while investigating > > >> https://bugzilla.redhat.com/show_bug.cgi?id=1707118 > > >> > > >> Found PCIe extended config space filled with random crap due to > > >> allocation being too small (conventional pci config space only). > > >> > > > > Can you amend this information to the commit description? > > > > <... > > > > >> PCI(e) config space is guest writable. Writes are limited by > > >> write mask (which probably is also filled with random stuff), > > > > > > Yes, it is also allocated with 256 bytes only. > > > > > >> so the guest can only flip enabled bits. But I suspect it > > >> still might be exploitable, so rather serious because it might > > >> be a host escape for the guest. On the other hand the device > > >> is probably not yet in widespread use. > > > > ...> > > I can add to the commit this paragraph of the cover letter, > and I think also the 'mitigation' note might as well go in. > > I've also put the cc:stable into the commit message. > > Updated commit, ready to apply to master if we're OK with it: > > https://git.linaro.org/people/peter.maydell/qemu-arm.git/commit/?h=staging&id=c075b5f318a8be628ab8edf93be33f5a93a4aacd Quoting new commit log: This makes sure the pci config space allocation is big enough, so accessing the PCIe extended config space doesn't overflow the pci config space buffer. PCI(e) config space is guest writable. Writes are limited bywrite mask (which probably is also filled with random stuff), so the guest can only flip enabled bits. But I suspect it still might be exploitable, so rather serious because it might be a host escape for the guest. On the other hand the device is probably not yet in widespread use. Mitigation: use "-device bochs-display" as conventional pci device only. Is it clear to others that this mitigation remark seems to be referencing an alternative configuration constraint to avoid the issue rather than what's actually implemented in this patch? IOW, if we never place the bochs-display device into a PCIe hierarchy, then extended config space is never accessible to the guest anyway, and there is no issue. I think this was meant to be an alternative to the patch but the enforcement of that would happen above QEMU, probably why it was mentioned in the cover letter rather than the original commit log. Thanks, Alex
Re: [Qemu-devel] [PATCH 0/1] display/bochs: fix pcie support (qemu security issue)
On 8/12/19 3:39 PM, Peter Maydell wrote: > On Mon, 12 Aug 2019 at 13:51, Philippe Mathieu-Daudé > wrote: >> >> On 8/12/19 2:45 PM, Paolo Bonzini wrote: >>> On 12/08/19 08:52, Gerd Hoffmann wrote: Just found while investigating https://bugzilla.redhat.com/show_bug.cgi?id=1707118 Found PCIe extended config space filled with random crap due to allocation being too small (conventional pci config space only). >> >> Can you amend this information to the commit description? >> >> <... >> PCI(e) config space is guest writable. Writes are limited by write mask (which probably is also filled with random stuff), >>> >>> Yes, it is also allocated with 256 bytes only. >>> so the guest can only flip enabled bits. But I suspect it still might be exploitable, so rather serious because it might be a host escape for the guest. On the other hand the device is probably not yet in widespread use. >> >> ...> > > I can add to the commit this paragraph of the cover letter, > and I think also the 'mitigation' note might as well go in. Yes. > > I've also put the cc:stable into the commit message. > > Updated commit, ready to apply to master if we're OK with it: > > https://git.linaro.org/people/peter.maydell/qemu-arm.git/commit/?h=staging&id=c075b5f318a8be628ab8edf93be33f5a93a4aacd Thank you! > > thanks > -- PMM >
Re: [Qemu-devel] [PATCH 0/1] display/bochs: fix pcie support (qemu security issue)
On Mon, 12 Aug 2019 at 13:51, Philippe Mathieu-Daudé wrote: > > On 8/12/19 2:45 PM, Paolo Bonzini wrote: > > On 12/08/19 08:52, Gerd Hoffmann wrote: > >> Just found while investigating > >> https://bugzilla.redhat.com/show_bug.cgi?id=1707118 > >> > >> Found PCIe extended config space filled with random crap due to > >> allocation being too small (conventional pci config space only). > >> > > Can you amend this information to the commit description? > > <... > > >> PCI(e) config space is guest writable. Writes are limited by > >> write mask (which probably is also filled with random stuff), > > > > Yes, it is also allocated with 256 bytes only. > > > >> so the guest can only flip enabled bits. But I suspect it > >> still might be exploitable, so rather serious because it might > >> be a host escape for the guest. On the other hand the device > >> is probably not yet in widespread use. > > ...> I can add to the commit this paragraph of the cover letter, and I think also the 'mitigation' note might as well go in. I've also put the cc:stable into the commit message. Updated commit, ready to apply to master if we're OK with it: https://git.linaro.org/people/peter.maydell/qemu-arm.git/commit/?h=staging&id=c075b5f318a8be628ab8edf93be33f5a93a4aacd thanks -- PMM
Re: [Qemu-devel] [PATCH 0/1] display/bochs: fix pcie support (qemu security issue)
On 8/12/19 2:45 PM, Paolo Bonzini wrote: > On 12/08/19 08:52, Gerd Hoffmann wrote: >> Just found while investigating >> https://bugzilla.redhat.com/show_bug.cgi?id=1707118 >> >> Found PCIe extended config space filled with random crap due to >> allocation being too small (conventional pci config space only). >> Can you amend this information to the commit description? <... >> PCI(e) config space is guest writable. Writes are limited by >> write mask (which probably is also filled with random stuff), > > Yes, it is also allocated with 256 bytes only. > >> so the guest can only flip enabled bits. But I suspect it >> still might be exploitable, so rather serious because it might >> be a host escape for the guest. On the other hand the device >> is probably not yet in widespread use. ...> >> Migitation: use "-device bochs-display" as conventional pci >> device only. >> >> Note: qemu 4.1 release is planned for tomorrow. >> >> Gerd Hoffmann (1): >> display/bochs: fix pcie support >> >> hw/display/bochs-display.c | 7 ++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> > > Looks good to me, and no other device seems to have the same issue. We > could add an assertion that pci_config_size has not increased after > calling pc->realize. > > Reviewed-by: Paolo Bonzini > > Paolo >
Re: [Qemu-devel] [PATCH 0/1] display/bochs: fix pcie support (qemu security issue)
On 12/08/19 08:52, Gerd Hoffmann wrote: > Just found while investigating > https://bugzilla.redhat.com/show_bug.cgi?id=1707118 > > Found PCIe extended config space filled with random crap due to > allocation being too small (conventional pci config space only). > > PCI(e) config space is guest writable. Writes are limited by > write mask (which probably is also filled with random stuff), Yes, it is also allocated with 256 bytes only. > so the guest can only flip enabled bits. But I suspect it > still might be exploitable, so rather serious because it might > be a host escape for the guest. On the other hand the device > is probably not yet in widespread use. > > Migitation: use "-device bochs-display" as conventional pci > device only. > > Note: qemu 4.1 release is planned for tomorrow. > > Gerd Hoffmann (1): > display/bochs: fix pcie support > > hw/display/bochs-display.c | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > Looks good to me, and no other device seems to have the same issue. We could add an assertion that pci_config_size has not increased after calling pc->realize. Reviewed-by: Paolo Bonzini Paolo
[Qemu-devel] [PATCH 0/1] display/bochs: fix pcie support (qemu security issue)
Just found while investigating https://bugzilla.redhat.com/show_bug.cgi?id=1707118 Found PCIe extended config space filled with random crap due to allocation being too small (conventional pci config space only). PCI(e) config space is guest writable. Writes are limited by write mask (which probably is also filled with random stuff), so the guest can only flip enabled bits. But I suspect it still might be exploitable, so rather serious because it might be a host escape for the guest. On the other hand the device is probably not yet in widespread use. Migitation: use "-device bochs-display" as conventional pci device only. Note: qemu 4.1 release is planned for tomorrow. Gerd Hoffmann (1): display/bochs: fix pcie support hw/display/bochs-display.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) -- 2.18.1
[Qemu-devel] [PATCH 0/1] Handle /proc/self/exe in execve
From: Olivier Dion
When the emulated process try to execve itself through /proc/self/exe,
QEMU user will be executed instead of the process.
The following short program demonstrated that:
--
#include
#include
#include
static char *ARGV0 = "STOP";
static char *ARGV1 = "-this-is-not-an-option";
int main(int argc, char *argv[], char *environ[])
{
(void)argc;
if (strcmp(argv[0], ARGV0) == 0)
return 0;
argv[0] = ARGV0;
argv[1] = ARGV1;
execve("/proc/self/exe", (char **const)argv,
(char **const)environ);
perror("execve");
return 1;
}
--
Will output:
--
qemu: unknown option 'this-is-not-an-option'
--
Olivier Dion (1):
linux-user: Handle /proc/self/exe in syscall execve
linux-user/syscall.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
--
2.22.0
[Qemu-devel] [PATCH 0/1] USB: bugfix on interrupt xfers with usb-redir
I have problem in xen with qemu xhci with usbredir backend. Windows bluetooth (BCM20703) driver does not work without proposed patch. Interrupt EP does not work as expected and described in USB spec. usb_20.pdf/5.7.3 Interrupt Transfer Packet Size Constraint: An endpoint must always transmit data payloads with a data field less than or equal to the endpoint’s wMaxPacketSize value. A device can move data via an interrupt pipe that is larger than wMaxPacketSize. A software client can accept this data via an IRP for the interrupt transfer that requires multiple bus transactions without requiring an IRP-complete notification per transaction. This can be achieved by specifying a buffer that can hold the desired data size. The size of the buffer is a multiple of wMaxPacketSize with some remainder. The endpoint must transfer each transaction except the last as wMaxPacketSize and the last transaction is the remainder. The multiple data transactions are moved over the bus at the period established for the pipe. When an interrupt transfer involves more data than can fit in one data payload of the currently established maximum size, all data payloads are required to be maximum-sized except for the last data payload, which will contain the remaining data. An interrupt transfer is complete when the endpoint does one of the following: • Has transferred exactly the amount of data expected • Transfers a packet with a payload size less than wMaxPacketSize or transfers a zero-length packet Examples of affected device on windows usbpcap decoded with wireshark: - snip of configuration descriptor: ENDPOINT DESCRIPTOR bLength: 7 bDescriptorType: 0x05 (ENDPOINT) bEndpointAddress: 0x81 IN Endpoint:1 1... = Direction: IN Endpoint 0001 = Endpoint Number: 0x1 bmAttributes: 0x03 ..11 = Transfertype: Interrupt-Transfer (0x3) wMaxPacketSize: 16 ...0 0... = Transactions per microframe: 1 (0) ..00 0001 = Maximum Packet Size: 16 bInterval: 1 - snip of two correct URB interrupts (len 70 and len 16) from non-virtualized communication and patched qemu: USB URB [Source: 1.6.1] [Destination: host] USBPcap pseudoheader length: 27 IRP ID: 0xa901ed380050 IRP USBD_STATUS: USBD_STATUS_SUCCESS (0x) URB Function: URB_FUNCTION_BULK_OR_INTERRUPT_TRANSFER (0x0009) IRP information: 0x01, Direction: PDO -> FDO 000. = Reserved: 0x00 ...1 = Direction: PDO -> FDO (0x1) URB bus id: 1 Device address: 6 Endpoint: 0x81, Direction: IN 1... = Direction: IN (1) 0001 = Endpoint number: 1 URB transfer type: URB_INTERRUPT (0x01) Packet Data Length: 70 [Request in: 43377] [Time from request: 0.006005000 seconds] [bInterfaceClass: Vendor Specific (0xff)] Leftover Capture Data: 0e4401021000ff03ccffefffec1ff20fe8fe3ff7... USB URB [Source: 1.6.1] [Destination: host] USBPcap pseudoheader length: 27 IRP ID: 0xa901ed380050 IRP USBD_STATUS: USBD_STATUS_SUCCESS (0x) URB Function: URB_FUNCTION_BULK_OR_INTERRUPT_TRANSFER (0x0009) IRP information: 0x01, Direction: PDO -> FDO 000. = Reserved: 0x00 ...1 = Direction: PDO -> FDO (0x1) URB bus id: 1 Device address: 6 Endpoint: 0x81, Direction: IN 1... = Direction: IN (1) 0001 = Endpoint number: 1 URB transfer type: URB_INTERRUPT (0x01) Packet Data Length: 16 [Request in: 43405] [Time from request: 0.002952000 seconds] [bInterfaceClass: Vendor Specific (0xff)] Leftover Capture Data: 0e0e01041102 - snip of the same two (more URB 70=16+16+16+16+6, 16=16+0) in actual qemu: USB URB [Source: 1.4.1] [Destination: host] USBPcap pseudoheader length: 27 IRP ID: 0xc5062ede69f0 IRP USBD_STATUS: USBD_STATUS_SUCCESS (0x) URB Function: URB_FUNCTION_BULK_OR_INTERRUPT_TRANSFER (0x0009) IRP information: 0x01, Direction: PDO -> FDO 000. = Reserved: 0x00 ...1 = Direction: PDO -> FDO (0x1) URB bus id: 1 Device address: 4 Endpoint: 0x81, Direction: IN 1... = Direction: IN (1) 0001 = Endpoint number: 1 URB transfer type: URB_INTERRUPT (0x01) Packet Data Length: 16 [Request in: 72930] [Time from request: 0.004881000 seconds] [bInterfaceClass: Vendor Specific (0xff)] Leftover Capture Data: 0e4401021000ff03ccffefff USB URB [Source: 1.4.1] [Destination: host] USBPcap pseudoheader length: 27 IRP ID: 0xc5062ede69f0 IRP USBD_STATUS: USBD_STATUS_SUCCESS (0x) URB Function: URB_FUNCTION_BULK_OR_INTERRUPT_TRANSFER (0x0009) IRP information: 0x01, Direction: PDO -> FDO 000. = Reserved: 0x00 ...1 = Direction: PDO -> FDO (0x1) URB bus id: 1 Device
[Qemu-devel] [PATCH 0/1] USB: bugfix on interrupt xfers with usb-redir
I have problem in xen with qemu xhci with usbredir backend. Windows bluetooth (BCM20703) driver does not work without proposed patch. Interrupt EP does not work as expected and described in USB spec. usb_20.pdf/5.7.3 Interrupt Transfer Packet Size Constraint: An endpoint must always transmit data payloads with a data field less than or equal to the endpoint’s wMaxPacketSize value. A device can move data via an interrupt pipe that is larger than wMaxPacketSize. A software client can accept this data via an IRP for the interrupt transfer that requires multiple bus transactions without requiring an IRP-complete notification per transaction. This can be achieved by specifying a buffer that can hold the desired data size. The size of the buffer is a multiple of wMaxPacketSize with some remainder. The endpoint must transfer each transaction except the last as wMaxPacketSize and the last transaction is the remainder. The multiple data transactions are moved over the bus at the period established for the pipe. When an interrupt transfer involves more data than can fit in one data payload of the currently established maximum size, all data payloads are required to be maximum-sized except for the last data payload, which will contain the remaining data. An interrupt transfer is complete when the endpoint does one of the following: • Has transferred exactly the amount of data expected • Transfers a packet with a payload size less than wMaxPacketSize or transfers a zero-length packet Examples of affected device on windows usbpcap decoded with wireshark: - snip of configuration descriptor: ENDPOINT DESCRIPTOR bLength: 7 bDescriptorType: 0x05 (ENDPOINT) bEndpointAddress: 0x81 IN Endpoint:1 1... = Direction: IN Endpoint 0001 = Endpoint Number: 0x1 bmAttributes: 0x03 ..11 = Transfertype: Interrupt-Transfer (0x3) wMaxPacketSize: 16 ...0 0... = Transactions per microframe: 1 (0) ..00 0001 = Maximum Packet Size: 16 bInterval: 1 - snip of two correct URB interrupts (len 70 and len 16) from non-virtualized communication and patched qemu: USB URB [Source: 1.6.1] [Destination: host] USBPcap pseudoheader length: 27 IRP ID: 0xa901ed380050 IRP USBD_STATUS: USBD_STATUS_SUCCESS (0x) URB Function: URB_FUNCTION_BULK_OR_INTERRUPT_TRANSFER (0x0009) IRP information: 0x01, Direction: PDO -> FDO 000. = Reserved: 0x00 ...1 = Direction: PDO -> FDO (0x1) URB bus id: 1 Device address: 6 Endpoint: 0x81, Direction: IN 1... = Direction: IN (1) 0001 = Endpoint number: 1 URB transfer type: URB_INTERRUPT (0x01) Packet Data Length: 70 [Request in: 43377] [Time from request: 0.006005000 seconds] [bInterfaceClass: Vendor Specific (0xff)] Leftover Capture Data: 0e4401021000ff03ccffefffec1ff20fe8fe3ff7... USB URB [Source: 1.6.1] [Destination: host] USBPcap pseudoheader length: 27 IRP ID: 0xa901ed380050 IRP USBD_STATUS: USBD_STATUS_SUCCESS (0x) URB Function: URB_FUNCTION_BULK_OR_INTERRUPT_TRANSFER (0x0009) IRP information: 0x01, Direction: PDO -> FDO 000. = Reserved: 0x00 ...1 = Direction: PDO -> FDO (0x1) URB bus id: 1 Device address: 6 Endpoint: 0x81, Direction: IN 1... = Direction: IN (1) 0001 = Endpoint number: 1 URB transfer type: URB_INTERRUPT (0x01) Packet Data Length: 16 [Request in: 43405] [Time from request: 0.002952000 seconds] [bInterfaceClass: Vendor Specific (0xff)] Leftover Capture Data: 0e0e01041102 - snip of the same two (more URB 70=16+16+16+16+6, 16=16+0) in actual qemu: USB URB [Source: 1.4.1] [Destination: host] USBPcap pseudoheader length: 27 IRP ID: 0xc5062ede69f0 IRP USBD_STATUS: USBD_STATUS_SUCCESS (0x) URB Function: URB_FUNCTION_BULK_OR_INTERRUPT_TRANSFER (0x0009) IRP information: 0x01, Direction: PDO -> FDO 000. = Reserved: 0x00 ...1 = Direction: PDO -> FDO (0x1) URB bus id: 1 Device address: 4 Endpoint: 0x81, Direction: IN 1... = Direction: IN (1) 0001 = Endpoint number: 1 URB transfer type: URB_INTERRUPT (0x01) Packet Data Length: 16 [Request in: 72930] [Time from request: 0.004881000 seconds] [bInterfaceClass: Vendor Specific (0xff)] Leftover Capture Data: 0e4401021000ff03ccffefff USB URB [Source: 1.4.1] [Destination: host] USBPcap pseudoheader length: 27 IRP ID: 0xc5062ede69f0 IRP USBD_STATUS: USBD_STATUS_SUCCESS (0x) URB Function: URB_FUNCTION_BULK_OR_INTERRUPT_TRANSFER (0x0009) IRP information: 0x01, Direction: PDO -> FDO 000. = Reserved: 0x00 ...1 = Direction: PDO -> FDO (0x1) URB bus id: 1 Device
Re: [Qemu-devel] [PATCH 0/1] Add check for header length in virtio-net-tx
On Tue, Jul 16, 2019 at 03:38:00AM +, Oleinik, Alexander wrote: > While fuzzing the virtio-net tx vq, I ran into an assertion failure due > to iov_copy offsets larger than the total iov size. Though there is > a check to cover this, it does not execute when !n->has_vnet_hdr. This > patch tries to fix this. > > The call stack for the assertion failure: > > #8 in __assert_fail (libc.so.6+0x300f1) > #9 in iov_copy iov.c:266:5 > #10 in virtio_net_flush_tx virtio-net.c:2073:23 > #11 in virtio_net_tx_bh virtio-net.c:2197:11 > #12 in aio_bh_poll async.c:118:13 > #13 in aio_dispatch aio-posix.c:460:5 > #14 in aio_ctx_dispatch async.c:261:5 > #15 in g_main_context_dispatch (libglib-2.0.so.0+0x4df2d) > #16 in glib_pollfds_poll main-loop.c:213:9 > #17 in os_host_main_loop_wait main-loop.c:236 > #18 in main_loop_wait main-loop.c:512 > #19 in virtio_net_tx_fuzz virtio-net-fuzz.c:160:3 > > Thanks > -Alex I think I'd rather introduce a variant of iov_copy that returns the remaining offset. the code that duplicates this for byte-swap seems to also have this problem. > Alexander Oleinik (1): > virtio-net: check guest header length is valid > > hw/net/virtio-net.c | 13 - > 1 file changed, 12 insertions(+), 1 deletion(-) > > -- > 2.20.1
[Qemu-devel] [PATCH 0/1] Add check for header length in virtio-net-tx
While fuzzing the virtio-net tx vq, I ran into an assertion failure due to iov_copy offsets larger than the total iov size. Though there is a check to cover this, it does not execute when !n->has_vnet_hdr. This patch tries to fix this. The call stack for the assertion failure: #8 in __assert_fail (libc.so.6+0x300f1) #9 in iov_copy iov.c:266:5 #10 in virtio_net_flush_tx virtio-net.c:2073:23 #11 in virtio_net_tx_bh virtio-net.c:2197:11 #12 in aio_bh_poll async.c:118:13 #13 in aio_dispatch aio-posix.c:460:5 #14 in aio_ctx_dispatch async.c:261:5 #15 in g_main_context_dispatch (libglib-2.0.so.0+0x4df2d) #16 in glib_pollfds_poll main-loop.c:213:9 #17 in os_host_main_loop_wait main-loop.c:236 #18 in main_loop_wait main-loop.c:512 #19 in virtio_net_tx_fuzz virtio-net-fuzz.c:160:3 Thanks -Alex Alexander Oleinik (1): virtio-net: check guest header length is valid hw/net/virtio-net.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) -- 2.20.1
Re: [Qemu-devel] [PATCH 0/1] RFC: don't obey the block device max transfer len / max segments for block devices
On Sun, 2019-06-30 at 18:08 +0300, Maxim Levitsky wrote: > It looks like Linux block devices, even in O_DIRECT mode don't have any user > visible > limit on transfer size / number of segments, which underlying block device > can have. > The block layer takes care of enforcing these limits by splitting the bios. > > By limiting the transfer sizes, we force qemu to do the splitting itself > which > introduces various overheads. > It is especially visible in nbd server, where the low max transfer size of the > underlying device forces us to advertise this over NBD, thus increasing the > traffic overhead in case of > image conversion which benefits from large blocks. > > More information can be found here: > https://bugzilla.redhat.com/show_bug.cgi?id=1647104 > > Tested this with qemu-img convert over nbd and natively and to my surprise, > even native IO performance improved a bit. > (The device on which it was tested is Intel Optane DC P4800X, which has 128k > max transfer size) > > The benchmark: > > Images were created using: > > Sparse image: qemu-img create -f qcow2 /dev/nvme0n1p3 1G / 10G / 100G > Allocated image: qemu-img create -f qcow2 /dev/nvme0n1p3 -o > preallocation=metadata 1G / 10G / 100G > > The test was: > > echo "convert native:" > rm -rf /dev/shm/disk.img > time qemu-img convert -p -f qcow2 -O raw -T none $FILE /dev/shm/disk.img > > /dev/zero > > echo "convert via nbd:" > qemu-nbd -k /tmp/nbd.sock -v -f qcow2 $FILE -x export --cache=none > --aio=native --fork > rm -rf /dev/shm/disk.img > time qemu-img convert -p -f raw -O raw > nbd:unix:/tmp/nbd.sock:exportname=export /dev/shm/disk.img > /dev/zero > > The results: > > = > 1G sparse image: > native: > before: 0.027s > after: 0.027s > nbd: > before: 0.287s > after: 0.035s > > = > 100G sparse image: > native: > before: 0.028s > after: 0.028s > nbd: > before: 23.796s > after: 0.109s > > = > 1G preallocated image: > native: >before: 0.454s >after: 0.427s > nbd: >before: 0.649s >after: 0.546s > > The block limits of max transfer size/max segment size are retained > for the SCSI passthrough because in this case the kernel passes the userspace > request > directly to the kernel scsi driver, bypassing the block layer, and thus there > is no code to split > such requests. > > What do you think? > > Fam, since you was the original author of the code that added > these limits, could you share your opinion on that? > What was the reason besides SCSI passthrough? > > Best regards, > Maxim Levitsky > > Maxim Levitsky (1): > raw-posix.c - use max transfer length / max segemnt count only for > SCSI passthrough > > block/file-posix.c | 16 +++- > 1 file changed, 7 insertions(+), 9 deletions(-) > Ping Best regards, Maxim Levitsky
[Qemu-devel] [PATCH 0/1] RFC: don't obey the block device max transfer len / max segments for block devices
It looks like Linux block devices, even in O_DIRECT mode don't have any user visible limit on transfer size / number of segments, which underlying block device can have. The block layer takes care of enforcing these limits by splitting the bios. By limiting the transfer sizes, we force qemu to do the splitting itself which introduces various overheads. It is especially visible in nbd server, where the low max transfer size of the underlying device forces us to advertise this over NBD, thus increasing the traffic overhead in case of image conversion which benefits from large blocks. More information can be found here: https://bugzilla.redhat.com/show_bug.cgi?id=1647104 Tested this with qemu-img convert over nbd and natively and to my surprise, even native IO performance improved a bit. (The device on which it was tested is Intel Optane DC P4800X, which has 128k max transfer size) The benchmark: Images were created using: Sparse image: qemu-img create -f qcow2 /dev/nvme0n1p3 1G / 10G / 100G Allocated image: qemu-img create -f qcow2 /dev/nvme0n1p3 -o preallocation=metadata 1G / 10G / 100G The test was: echo "convert native:" rm -rf /dev/shm/disk.img time qemu-img convert -p -f qcow2 -O raw -T none $FILE /dev/shm/disk.img > /dev/zero echo "convert via nbd:" qemu-nbd -k /tmp/nbd.sock -v -f qcow2 $FILE -x export --cache=none --aio=native --fork rm -rf /dev/shm/disk.img time qemu-img convert -p -f raw -O raw nbd:unix:/tmp/nbd.sock:exportname=export /dev/shm/disk.img > /dev/zero The results: = 1G sparse image: native: before: 0.027s after: 0.027s nbd: before: 0.287s after: 0.035s = 100G sparse image: native: before: 0.028s after: 0.028s nbd: before: 23.796s after: 0.109s = 1G preallocated image: native: before: 0.454s after: 0.427s nbd: before: 0.649s after: 0.546s The block limits of max transfer size/max segment size are retained for the SCSI passthrough because in this case the kernel passes the userspace request directly to the kernel scsi driver, bypassing the block layer, and thus there is no code to split such requests. What do you think? Fam, since you was the original author of the code that added these limits, could you share your opinion on that? What was the reason besides SCSI passthrough? Best regards, Maxim Levitsky Maxim Levitsky (1): raw-posix.c - use max transfer length / max segemnt count only for SCSI passthrough block/file-posix.c | 16 +++- 1 file changed, 7 insertions(+), 9 deletions(-) -- 2.17.2
[Qemu-devel] [PATCH 0/1] tcg: queued patch
The following changes since commit a050901d4b40092dc356b59912c6df39e389c7b9:
Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-4.1-20190612' into
staging (2019-06-12 14:43:47 +0100)
are available in the Git repository at:
https://github.com/rth7680/qemu.git tags/pull-tcg-20190612
for you to fetch changes up to 899f08ad1d1231dbbfa67298413f05ed2679fb02:
tcg: Fix typos in helper_gvec_sar{8,32,64}v (2019-06-12 21:08:38 -0700)
Fix vector arithmetic right shift helpers.
Richard Henderson (1):
tcg: Fix typos in helper_gvec_sar{8,32,64}v
accel/tcg/tcg-runtime-gvec.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[Qemu-devel] [PATCH 0/1] accel: get rid of AccelClass::opt_name
The AccelClass::opt_name is not used. Unless there is a reason for keeping hat attribute, this patch remove it. Git: https://github.com/wainersm/qemu Branch: accel_del_opt_name Travis: https://travis-ci.org/wainersm/qemu/builds/539721285 (Failed test case is not related with this change as well as fails on master too.) Wainer dos Santos Moschetta (1): accel: Remove unused AccelClass::opt_name attribute include/sysemu/accel.h | 1 - 1 file changed, 1 deletion(-) -- 2.21.0
[Qemu-devel] [PATCH 0/1] -accel should list enabled accelerators
On this series I changed the semantics of -accel help so that it shows only the accelerators enabled in the QEMU target binary. This behavior is now alike -cpu and -machine helps. Another reason for this proposal is that I am working on an improvement of Avocado QEMU framework which should skip tests tagged with, e.g, "accel:tcg" if tcg is not enabled in the binary. And it seems the best approach to detect the presence (or not) of an accelerator is to query QEMU with -accel help. Phillipe Mathieu-Daud?? proposed a similar fix [1] but it was never merged. My patch is slightly different but it implements some decisions that seemed consensus at that time: 1. Do not display qtest. It's an internal only accelerator. 2. It should display those that have support on the target binary, regardless if they are not present on the host. Example with this patch on x86_64 host (kvm not installed): --- $ configure --enable-kvm --enable-xen --target-list="x86_64-softmmu ppc64-softmmu" $ x86_64-softmmu/qemu-system-x86_64 -accel help Accelerators supported in QEMU binary: tcg xen kvm $ ppc64-softmmu/qemu-system-ppc64 -accel help Accelerators supported in QEMU binary: tcg --- Git: https://github.com/wainersm/qemu Branch: accel_list Travis: https://travis-ci.org/wainersm/qemu/builds/539366851 [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg491542.html Wainer dos Santos Moschetta (1): vl: make -accel help to list enabled accelerators only vl.c | 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) -- 2.21.0
[Qemu-devel] [PATCH 0/1] spapr: Do not re-read the clock on pre_save handler on migration
I suggest to remove the pre_save handler that saves the timebase before migrate. The commit that added this was ported from x86: 6053a86fe7bd: kvmclock: reduce kvmclock difference on migration The review [1] had a discussion about it. The author says that a VM already paused 10 minutes ago should re-read the clock just before migrate. But a reviewer question was not answered: "Is it really valid to make the clock move on an already-paused VM, only because it was migrated?" This clock move makes the guest know about the pause between the stop and migrate commands. Many side effects could happen after migration. [1] http://lists.nongnu.org/archive/html/qemu-devel/2016-12/msg00610.html Maxiwell S. Garcia (1): spapr: Do not re-read the clock on pre_save handler on migration hw/ppc/ppc.c | 24 1 file changed, 24 deletions(-) -- 2.20.1
[Qemu-devel] [PATCH 0/1] MAINTAINERS: Step in as maintainer for the parallel NOR flash devices
The parallel NOR flash models don't have a specific maintainer and default to the 'Block layer core' section. Step in to maintain them. The section still get covered by the Block layer team, but the idea is to offload them. The two devices are very similar (same technology), the difference is mostly the protocol to access them. Amusingly, between the two devices, the 'CFI01' which is used in enterprise grade products on ARM/X86 archs is the one that received the less care, while the 'CFI02' used by hobbyist boards is the more reliable. To some extent I plan to re-unify the models, and improve testing. I'm looking for co-maintainers or designated reviewers. (I asked Stephen Checkoway for help but unfortunately he can't). Any volunteer? Regards, Phil. PD: I Cc'ed all the people who made sinificant modification in the devices the last few years. Philippe Mathieu-Daudé (1): MAINTAINERS: Add an entry for the Parallel NOR Flash devices MAINTAINERS | 7 +++ 1 file changed, 7 insertions(+) -- 2.20.1
Re: [Qemu-devel] [PATCH 0/1] util/path: Do not cache all filenames at startup
Patchew URL: https://patchew.org/QEMU/20190417053225.27505-1-richard.hender...@linaro.org/ Hi, This series failed the docker-mingw@fedora build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash time make docker-test-mingw@fedora SHOW_ENV=1 J=14 NETWORK=1 === TEST SCRIPT END === CC replication.o CC block/raw-format.o /tmp/qemu-test/src/util/path.c: In function 'init_paths': /tmp/qemu-test/src/util/path.c:24:5: error: 'g_canonicalize_filename' is deprecated: Not available before 2.58 [-Werror=deprecated-declarations] base = g_canonicalize_filename(prefix, NULL); ^~~~ In file included from /usr/x86_64-w64-mingw32/sys-root/mingw/include/glib-2.0/glib.h:48, --- gchar *g_canonicalize_filename (const gchar *filename, ^~~ /tmp/qemu-test/src/util/path.c: In function 'path': /tmp/qemu-test/src/util/path.c:54:13: error: assignment discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers] ret = value ? value : name; ^ /tmp/qemu-test/src/util/path.c:60:9: error: 'g_canonicalize_filename' is deprecated: Not available before 2.58 [-Werror=deprecated-declarations] full_name = g_canonicalize_filename(g_path_skip_root(name), base); ^ In file included from /usr/x86_64-w64-mingw32/sys-root/mingw/include/glib-2.0/glib.h:48, --- /usr/x86_64-w64-mingw32/sys-root/mingw/include/glib-2.0/glib/gfileutils.h:176:8: note: declared here gchar *g_canonicalize_filename (const gchar *filename, ^~~ /tmp/qemu-test/src/util/path.c:74:17: error: assignment discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers] ret = name; ^ cc1: all warnings being treated as errors The full log is available at http://patchew.org/logs/20190417053225.27505-1-richard.hender...@linaro.org/testing.docker-mingw@fedora/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [Qemu-devel] [PATCH 0/1] util/path: Do not cache all filenames at startup
Patchew URL: https://patchew.org/QEMU/20190417053225.27505-1-richard.hender...@linaro.org/ Hi, This series failed the asan build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1 === TEST SCRIPT END === CC stubs/blockdev-close-all-bdrv-states.o CC stubs/clock-warp.o CC stubs/cpu-get-clock.o /tmp/qemu-test/src/util/path.c:24:12: error: 'g_canonicalize_filename' is deprecated [-Werror,-Wdeprecated-declarations] base = g_canonicalize_filename(prefix, NULL); ^ /usr/include/glib-2.0/glib/gfileutils.h:175:1: note: 'g_canonicalize_filename' has been explicitly marked deprecated here --- /usr/include/glib-2.0/glib/gmacros.h:432:37: note: expanded from macro 'G_DEPRECATED' #define G_DEPRECATED __attribute__((__deprecated__)) ^ /tmp/qemu-test/src/util/path.c:54:13: error: assigning to 'char *' from 'const void *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers] ret = value ? value : name; ^ /tmp/qemu-test/src/util/path.c:60:21: error: 'g_canonicalize_filename' is deprecated [-Werror,-Wdeprecated-declarations] full_name = g_canonicalize_filename(g_path_skip_root(name), base); ^ /usr/include/glib-2.0/glib/gfileutils.h:175:1: note: 'g_canonicalize_filename' has been explicitly marked deprecated here --- /usr/include/glib-2.0/glib/gmacros.h:432:37: note: expanded from macro 'G_DEPRECATED' #define G_DEPRECATED __attribute__((__deprecated__)) ^ /tmp/qemu-test/src/util/path.c:74:17: error: assigning to 'char *' from 'const char *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers] ret = name; ^ 4 errors generated. The full log is available at http://patchew.org/logs/20190417053225.27505-1-richard.hender...@linaro.org/testing.asan/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
[Qemu-devel] [PATCH 0/1] util/path: Do not cache all filenames at startup
This is a third variant attempting to fix the problem of the -L interp_prefix handling not coping well pointing to a full chroot. Previous versions include: https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg06592.html https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg07304.html Both of the other versions keep an open file descriptor to the directory that is interp_prefix. While this seems to be efficient, they also leak a file descriptor into the guest. This has follow-on effects on the emulation, causing failures e.g. within the LTP testsuite. This version uses only string manipulation and access(2). A plausible criticism of this is the mutex and the cache. Are they really needed? I have no idea. I can imagine removing that is actually more efficient, not needing a lock around a shared data structure. Comments? r~ Richard Henderson (1): util/path: Do not cache all filenames at startup util/path.c | 211 ++-- 1 file changed, 57 insertions(+), 154 deletions(-) -- 2.17.1
Re: [Qemu-devel] [PATCH 0/1] update copyright notice
On 26/03/19 08:14, IOMMU AUTHOR wrote: > got some clarification regarding this. > > i no longer wish to get this patch merged. Thanks very much for your understanding. Paolo
Re: [Qemu-devel] [PATCH 0/1] update copyright notice
On Wed, Mar 20, 2019 at 4:18 PM IOMMU AUTHOR wrote: > > > On Wed, Mar 20, 2019 at 12:16 AM IOMMU AUTHOR > wrote: > >> i'd rather the copyright notice on these files looks like i've put it >> below and since i just simply snipped my name(i'll provide legal proof >> that this is my name, if required), i don't expect this to be an issue. >> > > is this getting queued? cherry-picked? requests for v2, v3? any reviews? > > any ideas on how you can strip references to your name from a repo would > also be much appreciated. sadly this is the best i can do. > got some clarification regarding this. i no longer wish to get this patch merged. > > >> IOMMU AUTHOR (1): >> update copyright notice >> >> hw/i386/amd_iommu.c | 1 - >> hw/i386/amd_iommu.h | 1 - >> 2 files changed, 2 deletions(-) >> >> -- >> 2.21.0 >> >>
Re: [Qemu-devel] [PATCH 0/1] update copyright notice
On Wed, Mar 20, 2019 at 12:16 AM IOMMU AUTHOR wrote: > i'd rather the copyright notice on these files looks like i've put it > below and since i just simply snipped my name(i'll provide legal proof > that this is my name, if required), i don't expect this to be an issue. > is this getting queued? cherry-picked? requests for v2, v3? any reviews? any ideas on how you can strip references to your name from a repo would also be much appreciated. sadly this is the best i can do. > IOMMU AUTHOR (1): > update copyright notice > > hw/i386/amd_iommu.c | 1 - > hw/i386/amd_iommu.h | 1 - > 2 files changed, 2 deletions(-) > > -- > 2.21.0 > >
[Qemu-devel] [PATCH 0/1] update copyright notice
i'd rather the copyright notice on these files looks like i've put it below and since i just simply snipped my name(i'll provide legal proof that this is my name, if required), i don't expect this to be an issue. IOMMU AUTHOR (1): update copyright notice hw/i386/amd_iommu.c | 1 - hw/i386/amd_iommu.h | 1 - 2 files changed, 2 deletions(-) -- 2.21.0
Re: [Qemu-devel] [PATCH 0/1] chardev: support for authorization control on TLS connections
ping - soft freeze is less than a week away & I would like to see this in a chardev pull request in time for 4.0 On Wed, Feb 27, 2019 at 01:55:22PM +, Daniel P. Berrangé wrote: > This series provides the chardev part of the authorization control series > previously posted as: > > v1: https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg04482.html > v2: https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg05727.html > v3: https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg01639.html > v4: https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg04319.html > > The core authz framework is now merged & this patch has previously > had positive review, but I removed the r-b due to need for changes > to resolve merge conflicts with current chardev code. I expect this > is none the less ready for the chardev tree, should the maintainer > consider it acceptable. > > Daniel P. Berrangé (1): > chardev: add support for authorization for TLS clients > > chardev/char-socket.c | 12 +++- > chardev/char.c| 3 +++ > qapi/char.json| 6 ++ > qemu-options.hx | 9 +++-- > 4 files changed, 27 insertions(+), 3 deletions(-) > > -- > 2.20.1 > 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] [PATCH 0/1] migration: support for authorization control on TLS connections
ping - soft freeze is less than a week away & I'd like to see this merged for this release. On Wed, Feb 27, 2019 at 02:53:23PM +, Daniel P. Berrangé wrote: > This series provides the migration part of the authorization control series > previously posted as: > > v1: https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg04482.html > v2: https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg05727.html > v3: https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg01639.html > v4: https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg04319.html > > The core authz framework is now merged & this patch has had > positive review. Thus this migration part is ready to go into the > migration maintainer's tree, should the maintainer consider them > acceptable. > > Daniel P. Berrangé (1): > migration: add support for a "tls-authz" migration parameter > > hmp.c | 9 + > migration/migration.c | 8 > migration/tls.c | 2 +- > qapi/migration.json | 14 +- > 4 files changed, 31 insertions(+), 2 deletions(-) > > -- > 2.20.1 > 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 :|
[Qemu-devel] [PATCH 0/1] update copyright notice
i think it is best put as i've updated. David Kiarie (1): update copyright notice hw/i386/amd_iommu.c | 2 +- hw/i386/amd_iommu.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- 2.21.0
Re: [Qemu-devel] [PATCH 0/1] snip my name and email
Patchew URL: https://patchew.org/QEMU/20190221155359.8247-1-davidkiar...@gmail.com/ Hi, This series seems to have some coding style problems. See output below for more information: Message-id: 20190221155359.8247-1-davidkiar...@gmail.com Subject: [Qemu-devel] [PATCH 0/1] snip my name and email Type: series === 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 Switched to a new branch 'test' 3840647010 hw/i386: snip my name and email === OUTPUT BEGIN === ERROR: Missing Signed-off-by: line(s) total: 1 errors, 0 warnings, 14 lines checked Commit 384064701074 (hw/i386: snip my name and email) 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/20190221155359.8247-1-davidkiar...@gmail.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
[Qemu-devel] [PATCH 0/1] migration: support for authorization control on TLS connections
This series provides the migration part of the authorization control series previously posted as: v1: https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg04482.html v2: https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg05727.html v3: https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg01639.html v4: https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg04319.html The core authz framework is now merged & this patch has had positive review. Thus this migration part is ready to go into the migration maintainer's tree, should the maintainer consider them acceptable. Daniel P. Berrangé (1): migration: add support for a "tls-authz" migration parameter hmp.c | 9 + migration/migration.c | 8 migration/tls.c | 2 +- qapi/migration.json | 14 +- 4 files changed, 31 insertions(+), 2 deletions(-) -- 2.20.1
[Qemu-devel] [PATCH 0/1] chardev: support for authorization control on TLS connections
This series provides the chardev part of the authorization control series previously posted as: v1: https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg04482.html v2: https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg05727.html v3: https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg01639.html v4: https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg04319.html The core authz framework is now merged & this patch has previously had positive review, but I removed the r-b due to need for changes to resolve merge conflicts with current chardev code. I expect this is none the less ready for the chardev tree, should the maintainer consider it acceptable. Daniel P. Berrangé (1): chardev: add support for authorization for TLS clients chardev/char-socket.c | 12 +++- chardev/char.c| 3 +++ qapi/char.json| 6 ++ qemu-options.hx | 9 +++-- 4 files changed, 27 insertions(+), 3 deletions(-) -- 2.20.1
[Qemu-devel] [PATCH 0/1] update copyright notice
update copyright notice to reflect my full legal name. looks better to me that way. also, that way people are not under the impression i *own* qemu AMD IOMMU. thanks. David Kiarie (1): hw/i386: update copyright notice hw/i386/amd_iommu.c | 2 +- hw/i386/amd_iommu.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- 2.20.1
Re: [Qemu-devel] [PATCH 0/1] snip my name and email
On Thu, Feb 21, 2019 at 7:09 PM Jan Kiszka wrote: > On 21.02.19 17:05, Eric Blake wrote: > > On 2/21/19 9:53 AM, David Kiarie wrote: > >> the occurrence of my name and email on the files below may have led to > >> some confusion in the reporting of a few recent bugs. > >> > >> i have therefore choosen to snip it. > > > > Dropping an email from the copyright line makes sense; dropping the > > Copyright declaration altogether is a bit odd (the GPL works only in > > tandem with a copyright assertion) - but as you are the author of the > > line and copyright holder of your contributions, I am not in a position > > to say you are wrong in removing it, only that it looks odd. > > > > Yeah, indeed. > > David, also note that you probably have been addressed because > scripts/get_maintainer.pl will look into the git history of files that > some > patch addresses and pick up significant and/or recent contributors from > there. > There should be some opt-out statement from that, but I don't recall how. > > Jan, Eblake, i respect your opinion but i still think my original patch would have been best fit for the simple me. > Jan > > -- > Siemens AG, Corporate Technology, CT RDA IOT SES-DE > Corporate Competence Center Embedded Linux >
Re: [Qemu-devel] [PATCH 0/1] drop email from copyright declaraction
On Thu, Feb 21, 2019 at 8:04 PM David Kiarie wrote: > i personally mostly don't care what someone does with the code i wrote > but i mostly had this since everyone else was doing it but the presence > of the email on the file led to some recent confusion and i will > therefore drop it. > i think this makes the most sense as it simply allows Gabriel and Me to share copyright on the file which probably the most amicable solution with both of us having done a lot of work on the initial AMD IOMMU development and probably also the hugest contributions so far. > > thanks. > > David Kiarie (1): > hw/i386: drop my email from copyright declaration > > hw/i386/amd_iommu.c | 2 +- > hw/i386/amd_iommu.h | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > -- > 2.20.1 > >
Re: [Qemu-devel] [PATCH 0/1] snip my name and email
On Thu, Feb 21, 2019 at 8:35 PM Philippe Mathieu-Daudé wrote: > On 2/21/19 6:13 PM, Markus Armbruster wrote: > > > > > Can we resync with the kernel's script to get this feature? Or should > > we cherry-pick it? > > I think we are out-of-sync and only cherry-picking. > you can do this if you so wish but i am personally okay with the follow up patch i sent.
Re: [Qemu-devel] [PATCH 0/1] snip my name and email
On 2/21/19 6:13 PM, Markus Armbruster wrote: > Jan Kiszka writes: > >> On 21.02.19 17:48, Markus Armbruster wrote: >>> Jan Kiszka writes: >>> On 21.02.19 17:05, Eric Blake wrote: > On 2/21/19 9:53 AM, David Kiarie wrote: >> the occurrence of my name and email on the files below may have led to >> some confusion in the reporting of a few recent bugs. >> >> i have therefore choosen to snip it. > > Dropping an email from the copyright line makes sense; dropping the > Copyright declaration altogether is a bit odd (the GPL works only in > tandem with a copyright assertion) - but as you are the author of the > line and copyright holder of your contributions, I am not in a position > to say you are wrong in removing it, only that it looks odd. > Yeah, indeed. David, also note that you probably have been addressed because scripts/get_maintainer.pl will look into the git history of files that some patch addresses and pick up significant and/or recent contributors from there. >>> >>> MAINTAINERS covers these files, so the most common use of >>> get_maintainer.pl won't list anyone not listed there: >>> >>> $ scripts/get_maintainer.pl -f hw/i386/amd_iommu.[ch] >>> Paolo Bonzini (maintainer:X86) >>> Richard Henderson (maintainer:X86) >>> Eduardo Habkost (maintainer:X86) >>> "Michael S. Tsirkin" (supporter:PC) >>> Marcel Apfelbaum (supporter:PC) >>> qemu-devel@nongnu.org (open list:All patches CC here) >>> >>> David's contributions have aged out of --git: >>> >>> $ scripts/get_maintainer.pl -f hw/i386/amd_iommu.[ch] --git >>> "Michael S. Tsirkin" >>> (supporter:PC,commit_signer:12/7=100%,commit_signer:10/5=100%) >>> Marcel Apfelbaum (supporter:PC) >>> Paolo Bonzini (maintainer:X86) >>> Richard Henderson >>> (maintainer:X86,commit_signer:1/7=14%) >>> Eduardo Habkost (maintainer:X86) >>> Peter Xu >>> (commit_signer:6/7=86%,commit_signer:4/5=80%) >>> Brijesh Singh >>> (commit_signer:5/7=71%,commit_signer:4/5=80%) >>> "Alex Bennée" (commit_signer:1/7=14%) >>> Jan Kiszka (commit_signer:1/5=20%) >>> qemu-devel@nongnu.org (open list:All patches CC here) >>> >>> However, --git-blame still lists him: >>> >>> $ scripts/get_maintainer.pl -f hw/i386/amd_iommu.[ch] --git-blame >>> Paolo Bonzini (maintainer:X86,commits:7/24=29%) >>> Richard Henderson (maintainer:X86) >>> Eduardo Habkost (maintainer:X86,commits:7/24=29%) >>> "Michael S. Tsirkin" >>> (supporter:PC,commits:26/24=100%,commits:14/8=100%) >>> Marcel Apfelbaum (supporter:PC) >>> David Kiarie (authored >>> lines:1176/1645=71%,authored lines:274/373=73%) >>> Brijesh Singh (authored >>> lines:403/1645=24%,authored lines:93/373=25%,commits:4/8=50%) >>> Peter Xu (commits:10/24=42%,commits:4/8=50%) >>> David Gibson (commits:7/24=29%) >>> Jan Kiszka (commits:1/8=12%) >>> Prasad J Pandit (commits:1/8=12%) >>> qemu-devel@nongnu.org (open list:All patches CC here) >>> >>> --help admonishes: >>> >>>Using "--git-blame" is slow and may add old committers and authors >>>that are no longer active maintainers to the output. >>> There should be some opt-out statement from that, but I don't recall how. >>> >>> I don't think get_maintainer.pl supports a blacklist of people who don't >>> want to be pestered anymore. >>> >> >> # cat linux/.get_maintainer.ignore >> Christoph Hellwig >> >> That's why I remembered it vaguely. > > Ah! > > commit 435de0782b658c993350049e853ea9a8795df4e2 > Author: Joe Perches > Date: Thu Jun 25 15:01:50 2015 -0700 > > get_maintainer.pl: add .get_maintainer.ignore file capability > > Some people prefer not to be cc'd on patches. Add an ability to have a > file (.get_maintainer.ignore) with names and email addresses that are > excluded from being listed except when specifically listed as a maintainer > in a section. > > Signed-off-by: Joe Perches > Signed-off-by: Andrew Morton > Signed-off-by: Linus Torvalds > > Can we resync with the kernel's script to get this feature? Or should > we cherry-pick it? I think we are out-of-sync and only cherry-picking.
Re: [Qemu-devel] [PATCH 0/1] snip my name and email
Jan Kiszka writes: > On 21.02.19 17:48, Markus Armbruster wrote: >> Jan Kiszka writes: >> >>> On 21.02.19 17:05, Eric Blake wrote: On 2/21/19 9:53 AM, David Kiarie wrote: > the occurrence of my name and email on the files below may have led to > some confusion in the reporting of a few recent bugs. > > i have therefore choosen to snip it. Dropping an email from the copyright line makes sense; dropping the Copyright declaration altogether is a bit odd (the GPL works only in tandem with a copyright assertion) - but as you are the author of the line and copyright holder of your contributions, I am not in a position to say you are wrong in removing it, only that it looks odd. >>> >>> Yeah, indeed. >>> >>> David, also note that you probably have been addressed because >>> scripts/get_maintainer.pl will look into the git history of files that >>> some patch addresses and pick up significant and/or recent >>> contributors from there. >> >> MAINTAINERS covers these files, so the most common use of >> get_maintainer.pl won't list anyone not listed there: >> >> $ scripts/get_maintainer.pl -f hw/i386/amd_iommu.[ch] >> Paolo Bonzini (maintainer:X86) >> Richard Henderson (maintainer:X86) >> Eduardo Habkost (maintainer:X86) >> "Michael S. Tsirkin" (supporter:PC) >> Marcel Apfelbaum (supporter:PC) >> qemu-devel@nongnu.org (open list:All patches CC here) >> >> David's contributions have aged out of --git: >> >> $ scripts/get_maintainer.pl -f hw/i386/amd_iommu.[ch] --git >> "Michael S. Tsirkin" >> (supporter:PC,commit_signer:12/7=100%,commit_signer:10/5=100%) >> Marcel Apfelbaum (supporter:PC) >> Paolo Bonzini (maintainer:X86) >> Richard Henderson >> (maintainer:X86,commit_signer:1/7=14%) >> Eduardo Habkost (maintainer:X86) >> Peter Xu >> (commit_signer:6/7=86%,commit_signer:4/5=80%) >> Brijesh Singh >> (commit_signer:5/7=71%,commit_signer:4/5=80%) >> "Alex Bennée" (commit_signer:1/7=14%) >> Jan Kiszka (commit_signer:1/5=20%) >> qemu-devel@nongnu.org (open list:All patches CC here) >> >> However, --git-blame still lists him: >> >> $ scripts/get_maintainer.pl -f hw/i386/amd_iommu.[ch] --git-blame >> Paolo Bonzini (maintainer:X86,commits:7/24=29%) >> Richard Henderson (maintainer:X86) >> Eduardo Habkost (maintainer:X86,commits:7/24=29%) >> "Michael S. Tsirkin" >> (supporter:PC,commits:26/24=100%,commits:14/8=100%) >> Marcel Apfelbaum (supporter:PC) >> David Kiarie (authored >> lines:1176/1645=71%,authored lines:274/373=73%) >> Brijesh Singh (authored >> lines:403/1645=24%,authored lines:93/373=25%,commits:4/8=50%) >> Peter Xu (commits:10/24=42%,commits:4/8=50%) >> David Gibson (commits:7/24=29%) >> Jan Kiszka (commits:1/8=12%) >> Prasad J Pandit (commits:1/8=12%) >> qemu-devel@nongnu.org (open list:All patches CC here) >> >> --help admonishes: >> >>Using "--git-blame" is slow and may add old committers and authors >>that are no longer active maintainers to the output. >> >>> There should be some opt-out statement from >>> that, but I don't recall how. >> >> I don't think get_maintainer.pl supports a blacklist of people who don't >> want to be pestered anymore. >> > > # cat linux/.get_maintainer.ignore > Christoph Hellwig > > That's why I remembered it vaguely. Ah! commit 435de0782b658c993350049e853ea9a8795df4e2 Author: Joe Perches Date: Thu Jun 25 15:01:50 2015 -0700 get_maintainer.pl: add .get_maintainer.ignore file capability Some people prefer not to be cc'd on patches. Add an ability to have a file (.get_maintainer.ignore) with names and email addresses that are excluded from being listed except when specifically listed as a maintainer in a section. Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds Can we resync with the kernel's script to get this feature? Or should we cherry-pick it?
[Qemu-devel] [PATCH 0/1] drop email from copyright declaraction
i personally mostly don't care what someone does with the code i wrote but i mostly had this since everyone else was doing it but the presence of the email on the file led to some recent confusion and i will therefore drop it. thanks. David Kiarie (1): hw/i386: drop my email from copyright declaration hw/i386/amd_iommu.c | 2 +- hw/i386/amd_iommu.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- 2.20.1
Re: [Qemu-devel] [PATCH 0/1] snip my name and email
i will just drop the email. thanks.
Re: [Qemu-devel] [PATCH 0/1] snip my name and email
On 21.02.19 17:48, Markus Armbruster wrote: Jan Kiszka writes: On 21.02.19 17:05, Eric Blake wrote: On 2/21/19 9:53 AM, David Kiarie wrote: the occurrence of my name and email on the files below may have led to some confusion in the reporting of a few recent bugs. i have therefore choosen to snip it. Dropping an email from the copyright line makes sense; dropping the Copyright declaration altogether is a bit odd (the GPL works only in tandem with a copyright assertion) - but as you are the author of the line and copyright holder of your contributions, I am not in a position to say you are wrong in removing it, only that it looks odd. Yeah, indeed. David, also note that you probably have been addressed because scripts/get_maintainer.pl will look into the git history of files that some patch addresses and pick up significant and/or recent contributors from there. MAINTAINERS covers these files, so the most common use of get_maintainer.pl won't list anyone not listed there: $ scripts/get_maintainer.pl -f hw/i386/amd_iommu.[ch] Paolo Bonzini (maintainer:X86) Richard Henderson (maintainer:X86) Eduardo Habkost (maintainer:X86) "Michael S. Tsirkin" (supporter:PC) Marcel Apfelbaum (supporter:PC) qemu-devel@nongnu.org (open list:All patches CC here) David's contributions have aged out of --git: $ scripts/get_maintainer.pl -f hw/i386/amd_iommu.[ch] --git "Michael S. Tsirkin" (supporter:PC,commit_signer:12/7=100%,commit_signer:10/5=100%) Marcel Apfelbaum (supporter:PC) Paolo Bonzini (maintainer:X86) Richard Henderson (maintainer:X86,commit_signer:1/7=14%) Eduardo Habkost (maintainer:X86) Peter Xu (commit_signer:6/7=86%,commit_signer:4/5=80%) Brijesh Singh (commit_signer:5/7=71%,commit_signer:4/5=80%) "Alex Bennée" (commit_signer:1/7=14%) Jan Kiszka (commit_signer:1/5=20%) qemu-devel@nongnu.org (open list:All patches CC here) However, --git-blame still lists him: $ scripts/get_maintainer.pl -f hw/i386/amd_iommu.[ch] --git-blame Paolo Bonzini (maintainer:X86,commits:7/24=29%) Richard Henderson (maintainer:X86) Eduardo Habkost (maintainer:X86,commits:7/24=29%) "Michael S. Tsirkin" (supporter:PC,commits:26/24=100%,commits:14/8=100%) Marcel Apfelbaum (supporter:PC) David Kiarie (authored lines:1176/1645=71%,authored lines:274/373=73%) Brijesh Singh (authored lines:403/1645=24%,authored lines:93/373=25%,commits:4/8=50%) Peter Xu (commits:10/24=42%,commits:4/8=50%) David Gibson (commits:7/24=29%) Jan Kiszka (commits:1/8=12%) Prasad J Pandit (commits:1/8=12%) qemu-devel@nongnu.org (open list:All patches CC here) --help admonishes: Using "--git-blame" is slow and may add old committers and authors that are no longer active maintainers to the output. There should be some opt-out statement from that, but I don't recall how. I don't think get_maintainer.pl supports a blacklist of people who don't want to be pestered anymore. # cat linux/.get_maintainer.ignore Christoph Hellwig That's why I remembered it vaguely. Jan -- Siemens AG, Corporate Technology, CT RDA IOT SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 0/1] snip my name and email
Jan Kiszka writes: > On 21.02.19 17:05, Eric Blake wrote: >> On 2/21/19 9:53 AM, David Kiarie wrote: >>> the occurrence of my name and email on the files below may have led to >>> some confusion in the reporting of a few recent bugs. >>> >>> i have therefore choosen to snip it. >> >> Dropping an email from the copyright line makes sense; dropping the >> Copyright declaration altogether is a bit odd (the GPL works only in >> tandem with a copyright assertion) - but as you are the author of the >> line and copyright holder of your contributions, I am not in a position >> to say you are wrong in removing it, only that it looks odd. >> > > Yeah, indeed. > > David, also note that you probably have been addressed because > scripts/get_maintainer.pl will look into the git history of files that > some patch addresses and pick up significant and/or recent > contributors from there. MAINTAINERS covers these files, so the most common use of get_maintainer.pl won't list anyone not listed there: $ scripts/get_maintainer.pl -f hw/i386/amd_iommu.[ch] Paolo Bonzini (maintainer:X86) Richard Henderson (maintainer:X86) Eduardo Habkost (maintainer:X86) "Michael S. Tsirkin" (supporter:PC) Marcel Apfelbaum (supporter:PC) qemu-devel@nongnu.org (open list:All patches CC here) David's contributions have aged out of --git: $ scripts/get_maintainer.pl -f hw/i386/amd_iommu.[ch] --git "Michael S. Tsirkin" (supporter:PC,commit_signer:12/7=100%,commit_signer:10/5=100%) Marcel Apfelbaum (supporter:PC) Paolo Bonzini (maintainer:X86) Richard Henderson (maintainer:X86,commit_signer:1/7=14%) Eduardo Habkost (maintainer:X86) Peter Xu (commit_signer:6/7=86%,commit_signer:4/5=80%) Brijesh Singh (commit_signer:5/7=71%,commit_signer:4/5=80%) "Alex Bennée" (commit_signer:1/7=14%) Jan Kiszka (commit_signer:1/5=20%) qemu-devel@nongnu.org (open list:All patches CC here) However, --git-blame still lists him: $ scripts/get_maintainer.pl -f hw/i386/amd_iommu.[ch] --git-blame Paolo Bonzini (maintainer:X86,commits:7/24=29%) Richard Henderson (maintainer:X86) Eduardo Habkost (maintainer:X86,commits:7/24=29%) "Michael S. Tsirkin" (supporter:PC,commits:26/24=100%,commits:14/8=100%) Marcel Apfelbaum (supporter:PC) David Kiarie (authored lines:1176/1645=71%,authored lines:274/373=73%) Brijesh Singh (authored lines:403/1645=24%,authored lines:93/373=25%,commits:4/8=50%) Peter Xu (commits:10/24=42%,commits:4/8=50%) David Gibson (commits:7/24=29%) Jan Kiszka (commits:1/8=12%) Prasad J Pandit (commits:1/8=12%) qemu-devel@nongnu.org (open list:All patches CC here) --help admonishes: Using "--git-blame" is slow and may add old committers and authors that are no longer active maintainers to the output. > There should be some opt-out statement from > that, but I don't recall how. I don't think get_maintainer.pl supports a blacklist of people who don't want to be pestered anymore.
Re: [Qemu-devel] [PATCH 0/1] snip my name and email
On 21.02.19 17:05, Eric Blake wrote: On 2/21/19 9:53 AM, David Kiarie wrote: the occurrence of my name and email on the files below may have led to some confusion in the reporting of a few recent bugs. i have therefore choosen to snip it. Dropping an email from the copyright line makes sense; dropping the Copyright declaration altogether is a bit odd (the GPL works only in tandem with a copyright assertion) - but as you are the author of the line and copyright holder of your contributions, I am not in a position to say you are wrong in removing it, only that it looks odd. Yeah, indeed. David, also note that you probably have been addressed because scripts/get_maintainer.pl will look into the git history of files that some patch addresses and pick up significant and/or recent contributors from there. There should be some opt-out statement from that, but I don't recall how. Jan -- Siemens AG, Corporate Technology, CT RDA IOT SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 0/1] snip my name and email
On 2/21/19 9:53 AM, David Kiarie wrote: > the occurrence of my name and email on the files below may have led to > some confusion in the reporting of a few recent bugs. > > i have therefore choosen to snip it. Dropping an email from the copyright line makes sense; dropping the Copyright declaration altogether is a bit odd (the GPL works only in tandem with a copyright assertion) - but as you are the author of the line and copyright holder of your contributions, I am not in a position to say you are wrong in removing it, only that it looks odd. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
[Qemu-devel] [PATCH 0/1] snip my name and email
the occurrence of my name and email on the files below may have led to some confusion in the reporting of a few recent bugs. i have therefore choosen to snip it. David Kiarie (1): hw/i386: snip my name and email hw/i386/amd_iommu.c | 1 - hw/i386/amd_iommu.h | 1 - 2 files changed, 2 deletions(-) -- 2.20.1
[Qemu-devel] [PATCH 0/1] Introduce a Python module structure
The amount of Python code that is being reused by a now large number
of different scripts and tests on QEMU urges for a better structure.
This addresses the feedback received on a previous RFC[1], but further
changes that will really benefit from this change were not attempted
here. Once, the module structure is present, I myself have plans to
send some unittests to "QEMUMachine", to some parts of "qemu.qmp", and
others.
Documentation, lint and style checkers are other possible candidates.
[1] - https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg05478.html
---
Git Info:
- URI: https://github.com/clebergnu/qemu/tree/sent/python_module
- Remote: https://github.com/clebergnu/qemu
- Branch: sent/python_module
Travis CI Info:
- Job: https://travis-ci.org/clebergnu/qemu/builds/489616874
Cleber Rosa (1):
Introduce a Python module structure
configure | 1 +
scripts/qemu.py => python/qemu/__init__.py | 11 ++-
{scripts/qmp => python/qemu}/qmp.py| 0
{scripts => python/qemu}/qtest.py | 5 +++--
scripts/device-crash-test | 2 ++
scripts/qmp/__init__.py| 0
scripts/qmp/qemu-ga-client | 5 -
scripts/qmp/qmp-shell | 4 +++-
scripts/render_block_graph.py | 2 ++
tests/acceptance/avocado_qemu/__init__.py | 5 ++---
tests/acceptance/virtio_version.py | 2 +-
tests/migration/guestperf/engine.py| 7 ---
tests/qemu-iotests/235 | 2 +-
tests/qemu-iotests/238 | 2 +-
tests/qemu-iotests/iotests.py | 4 ++--
tests/vm/basevm.py | 2 +-
16 files changed, 33 insertions(+), 21 deletions(-)
rename scripts/qemu.py => python/qemu/__init__.py (98%)
rename {scripts/qmp => python/qemu}/qmp.py (100%)
rename {scripts => python/qemu}/qtest.py (98%)
delete mode 100644 scripts/qmp/__init__.py
--
2.20.1
Re: [Qemu-devel] [PATCH 0/1] migration: calculate expected_downtime considering redirtied ram
Patchew URL: https://patchew.org/QEMU/20190122150543.16889-1-bal...@linux.vnet.ibm.com/ Hi, This series failed the docker-mingw@fedora build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash time make docker-test-mingw@fedora SHOW_ENV=1 J=14 === TEST SCRIPT END === CC net/util.o CC net/hub.o /tmp/qemu-test/src/migration/migration.c: In function 'migration_update_counters': /tmp/qemu-test/src/migration/migration.c:2907:9: error: 'remaining_ram_transfer_time' undeclared (first use in this function) remaining_ram_transfer_time = ram_counters.remaining / bandwidth ^~~ /tmp/qemu-test/src/migration/migration.c:2907:9: note: each undeclared identifier is reported only once for each function it appears in /tmp/qemu-test/src/migration/migration.c:2907:73: error: expected ';' before 'newly_dirtied_ram' remaining_ram_transfer_time = ram_counters.remaining / bandwidth ^ ; The full log is available at http://patchew.org/logs/20190122150543.16889-1-bal...@linux.vnet.ibm.com/testing.docker-mingw@fedora/?type=message. --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [Qemu-devel] [PATCH 0/1] target/arm: Amend erroneous BRAA/BLRAA opcode check
On Wed, 30 Jan 2019 at 20:22, Mathew Maidment wrote: > This is a patch that fixes a condition within disas_uncond_b_reg() related to > BRAA and BLRAA that would always result in the unallocated encoding path being > taken. > > Hopefully everything is in order. This is only my second patch, so if anything > is wrong, that's my bad. > > Thanks in advance for reviewing =) Hi -- thanks for this patch. I think this bug is already covered by this patch that went onto the mailing list last week: https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg06728.html -- you might like to test that series. thanks -- PMM
[Qemu-devel] [PATCH 0/1] target/arm: Amend erroneous BRAA/BLRAA opcode check
Hi, This is a patch that fixes a condition within disas_uncond_b_reg() related to BRAA and BLRAA that would always result in the unallocated encoding path being taken. Hopefully everything is in order. This is only my second patch, so if anything is wrong, that's my bad. Thanks in advance for reviewing =) Mathew Maidment (1): target/arm: Amend erroneous BRAA/BLRAA opcode check target/arm/translate-a64.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.17.2 (Apple Git-113)
Re: [Qemu-devel] [PATCH 0/1] block: Eliminate the S_1KiB, S_2KiB, ... macros
Am 11.01.2019 um 20:14 hat Markus Armbruster geschrieben: > Back in September, Leonid Block added a whole bunch of macros (commit > 540b8492618) to improve readability of qcow2.h a bit (commit > b6a95c6d100). He later used them to fix the "vdi" driver's parameter > cluster_size's default value (commit 3dd5b8f4718). He has now > proposed a further patch[1] to auto-generate these macros. That patch > feels overengineered to me. > > On closer examination, I found I dislike the macros before his new > patch. So did Eric Blake. > > The macros exist because the common KiB, MiB, ... macros aren't usable > when you need a literal rather than a constant expression. > stringify() does, and we use it to define the QemuOpts default value. > > Eric proposed to improve QemuOpts to accept integer default values, > too[2]. Before I review that patch series, I want to establish a > "stupidest solution that can possibly work" baseline. And that's what > this patch is. > > [1] [PATCH v2 0/1] include: Auto-generate the sizes lookup table > Message-ID: <20190103213320.2653-1-lbl...@janustech.com> > > [2] [PATCH v3 0/6] include: Auto-generate the sizes lookup table > Message-Id: <20190110191901.5082-1-ebl...@redhat.com> Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] [PATCH 0/1] input-linux: customizable grab toggle keys v5
On Wed, Jan 23, 2019 at 04:45:54PM -0500, Ryan El Kochta wrote: > This patch adds a new option to the input-linux object: > > grab-toggle=[key-combo] Other way around: commit message describing the patch goes to the patch (1/1), the series description (and changes to previous revisions if any) go to the cover letter (0/1). Picked up the patch, copied over the description from the cover letter. thanks, Gerd
[Qemu-devel] [PATCH 0/1] input-linux: customizable grab toggle keys v5
This patch adds a new option to the input-linux object: grab-toggle=[key-combo] The key combination can be one of the following: * ctrl-ctrl * alt-alt * meta-meta * scrolllock * ctrl-scrolllock The user can pick any of these key combinations. The VM's grab of the evdev device will be toggled when the key combination is pressed. Any invalid setting will result in an error. No setting will result in the current default of ctrl-ctrl. The right and left ctrl key both work for Ctrl-Scrolllock. If scrolllock is selected as one of the grab-toggle keys, it will be entirely disabled and not passed to the guest at all. This is to prevent enabling it while attempting to leave or enter the VM. On the host, scrolllock can be disabled using xmodmap. First, find the modifier that Scroll_Lock is bound to: $ xmodmap -pm Then, remove Scroll_Lock from it, replacing modX with the modifier: $ xmodmap -e 'remove modX = Scroll_Lock' If Scroll_Lock is not bound to any modifier, it is already disabled. To save the changes, add them to your xinitrc. Ryan El Kochta (1): input-linux: customizable grab toggle keys v5 qapi/ui.json | 10 ui/input-linux.c | 66 +--- 2 files changed, 73 insertions(+), 3 deletions(-) -- 2.20.1
[Qemu-devel] [PATCH 0/1] migration: calculate expected_downtime considering redirtied ram
From: Balamuruhan S Based on the discussion with Dave and David Gibson earlier with respect to expected_downtime calculation, https://lists.gnu.org/archive/html/qemu-devel/2018-04/msg02418.html got suggestions that the calculation is of not accurate and we need to consider the ram that gets redirtied during the time when we would have actually transferred ram in the current iteration. so I have came up with a calculation by considering the ram that could get redirtied during the current iteration at the time we would have transferred the remaining ram in current iteration. By this way, the total ram to be transferred will be remaining ram + redirtied ram and dividing with bandwidth would yield us better expected_downtime value. Please help to review and suggest about this approach. Balamuruhan S (1): migration: calculate expected_downtime considering redirtied ram migration/migration.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) -- 2.14.5
Re: [Qemu-devel] [PATCH 0/1] block: Eliminate the S_1KiB, S_2KiB, ... macros
Leonid Bloch writes: > On 1/11/19 9:14 PM, Markus Armbruster wrote: >> Back in September, Leonid Block added a whole bunch of macros (commit > > * Bloch. :) I apologize for my carelessness. Explanation, no excuse: $ git-log --author=armbru -Sblock -i --oneline | wc -l 167 $ git-log --author=armbru -Sbloch -i --oneline | wc -l 0 Déformation professionnelle... [...]
Re: [Qemu-devel] [PATCH 0/1] block: Eliminate the S_1KiB, S_2KiB, ... macros
On 1/11/19 9:14 PM, Markus Armbruster wrote: > Back in September, Leonid Block added a whole bunch of macros (commit * Bloch. :) > 540b8492618) to improve readability of qcow2.h a bit (commit > b6a95c6d100). He later used them to fix the "vdi" driver's parameter > cluster_size's default value (commit 3dd5b8f4718). He has now > proposed a further patch[1] to auto-generate these macros. That patch > feels overengineered to me. > > On closer examination, I found I dislike the macros before his new > patch. So did Eric Blake. > > The macros exist because the common KiB, MiB, ... macros aren't usable > when you need a literal rather than a constant expression. > stringify() does, and we use it to define the QemuOpts default value. > > Eric proposed to improve QemuOpts to accept integer default values, > too[2]. Before I review that patch series, I want to establish a > "stupidest solution that can possibly work" baseline. And that's what > this patch is. > > [1] [PATCH v2 0/1] include: Auto-generate the sizes lookup table > Message-ID: <20190103213320.2653-1-lbl...@janustech.com> > > [2] [PATCH v3 0/6] include: Auto-generate the sizes lookup table > Message-Id: <20190110191901.5082-1-ebl...@redhat.com> > > Markus Armbruster (1): >block: Eliminate the S_1KiB, S_2KiB, ... macros > > block/qcow2.h| 10 +++--- > block/vdi.c | 3 +- > include/qemu/units.h | 73 > 3 files changed, 7 insertions(+), 79 deletions(-) >
[Qemu-devel] [PATCH 0/1] block: Eliminate the S_1KiB, S_2KiB, ... macros
Back in September, Leonid Block added a whole bunch of macros (commit 540b8492618) to improve readability of qcow2.h a bit (commit b6a95c6d100). He later used them to fix the "vdi" driver's parameter cluster_size's default value (commit 3dd5b8f4718). He has now proposed a further patch[1] to auto-generate these macros. That patch feels overengineered to me. On closer examination, I found I dislike the macros before his new patch. So did Eric Blake. The macros exist because the common KiB, MiB, ... macros aren't usable when you need a literal rather than a constant expression. stringify() does, and we use it to define the QemuOpts default value. Eric proposed to improve QemuOpts to accept integer default values, too[2]. Before I review that patch series, I want to establish a "stupidest solution that can possibly work" baseline. And that's what this patch is. [1] [PATCH v2 0/1] include: Auto-generate the sizes lookup table Message-ID: <20190103213320.2653-1-lbl...@janustech.com> [2] [PATCH v3 0/6] include: Auto-generate the sizes lookup table Message-Id: <20190110191901.5082-1-ebl...@redhat.com> Markus Armbruster (1): block: Eliminate the S_1KiB, S_2KiB, ... macros block/qcow2.h| 10 +++--- block/vdi.c | 3 +- include/qemu/units.h | 73 3 files changed, 7 insertions(+), 79 deletions(-) -- 2.17.2
[Qemu-devel] [PATCH 0/1] include: Auto-generate the sizes lookup table
Following the conversations here: https://patchwork.kernel.org/patch/10665157 and here: https://patchwork.kernel.org/patch/10666975 Making the lookup table for power-of-two sizes auto-generated, instead of being hard-coded into the units.h file. I'm not sure if the changes I've made to Makefile here are "standard". Please correct me if that's not the case. Sorry it took so much time - I was busy with something completely different. Regards, Leonid. Leonid Bloch (1): include: Auto-generate the sizes lookup table .gitignore | 1 + Makefile | 5 +++ block/qcow2.h| 2 +- block/vdi.c | 1 + include/qemu/units.h | 73 scripts/gen-sizes.sh | 66 +++ 6 files changed, 74 insertions(+), 74 deletions(-) create mode 100755 scripts/gen-sizes.sh -- 2.20.1
Re: [Qemu-devel] [PATCH 0/1] checkpatch: checker for comment block
Peter Maydell writes: > On Fri, 14 Dec 2018 at 12:31, Markus Armbruster wrote: >> Peter Maydell writes: >> > On Fri, 14 Dec 2018 at 06:29, Markus Armbruster wrote: >> > I have to admit I never really understood what tweak >> > you wanted making to the commit message. I'm happy >> > to make it clearer: do you want to suggest a rewording? >> >> The commit message claims "(The only changes needed are that Linux's >> checkpatch.pl WARN() function takes an extra argument that ours does >> not.)" This isn't the case. You also dropped the kernel's "Networking >> with an initial /*" special case. > > The bit of the commit message you didn't quote says > "by backporting the relevant > parts of the Linux kernel's checkpatch.pl. (The only changes > needed are that Linux's checkpatch.pl WARN() function takes > an extra argument that ours does not.)" > > The networking special case is not a "relevant part", which > is why it's not in the patch. The bracketed statement applies > to the code in the patch, not any lumps of code in the > kernel's checkpatch that are not in the patch. I understand you logic now. > Anyway, I've adjusted the commit message as you suggest. > > Since we seem to now have consensus on the checkpatch patch, > I'm going to put it into the "misc" pull request I'm putting > together. Thanks!
Re: [Qemu-devel] [PATCH 0/1] checkpatch: checker for comment block
On Fri, 14 Dec 2018 at 12:31, Markus Armbruster wrote: > Peter Maydell writes: > > On Fri, 14 Dec 2018 at 06:29, Markus Armbruster wrote: > > I have to admit I never really understood what tweak > > you wanted making to the commit message. I'm happy > > to make it clearer: do you want to suggest a rewording? > > The commit message claims "(The only changes needed are that Linux's > checkpatch.pl WARN() function takes an extra argument that ours does > not.)" This isn't the case. You also dropped the kernel's "Networking > with an initial /*" special case. The bit of the commit message you didn't quote says "by backporting the relevant parts of the Linux kernel's checkpatch.pl. (The only changes needed are that Linux's checkpatch.pl WARN() function takes an extra argument that ours does not.)" The networking special case is not a "relevant part", which is why it's not in the patch. The bracketed statement applies to the code in the patch, not any lumps of code in the kernel's checkpatch that are not in the patch. Anyway, I've adjusted the commit message as you suggest. Since we seem to now have consensus on the checkpatch patch, I'm going to put it into the "misc" pull request I'm putting together. thanks -- PMM
Re: [Qemu-devel] [PATCH 0/1] checkpatch: checker for comment block
Peter Maydell writes: > On Fri, 14 Dec 2018 at 06:29, Markus Armbruster wrote: >> >> Paolo Bonzini writes: >> >> > On 13/12/18 19:21, Peter Maydell wrote: >> >> On Thu, 13 Dec 2018 at 18:07, Paolo Bonzini wrote: >> >>> On 13/12/18 19:01, Peter Maydell wrote: >> I sent a patch to do this a little while back: >> https://patchwork.kernel.org/patch/10561557/ > >> Personally I think we should just commit my patch, and then >> we can stop having people manually pointing out where >> submitters' patches don't match CODING_STYLE. >> >> Concur. It has my R-by, modulo a commit message tweak. > > I have to admit I never really understood what tweak > you wanted making to the commit message. I'm happy > to make it clearer: do you want to suggest a rewording? The commit message claims "(The only changes needed are that Linux's checkpatch.pl WARN() function takes an extra argument that ours does not.)" This isn't the case. You also dropped the kernel's "Networking with an initial /*" special case. The simplest way to fix an incorrect claim is to delete it :) If you want to explain what you changed, you could say something like "(The only changes needed are that Linux's checkpatch.pl WARN() function takes an extra argument that ours does not, and the kernel has a special case for networking code we don't want.)"
Re: [Qemu-devel] [PATCH 0/1] checkpatch: checker for comment block
On 12/13/2018 04:01 PM, Peter Maydell wrote: On Thu, 13 Dec 2018 at 17:57, Wainer dos Santos Moschetta wrote: Eduardo Habkost pointed out a malformed block of comments on my patch [1] that I had ran checkpatch.pl and no warn/error was reported. Then I realized the script does not catch such as case (or it had a bug). It turns out that checkpatch.pl does not parse comment blocks (If I understood its code correctly...). So I implemented a checker that warns about: 1. block doesn't begin on its own line. Example: /* blah blah * and blah blah */ I sent a patch to do this a little while back: https://patchwork.kernel.org/patch/10561557/ Self-NACK my patch in favor of that, which has additional checks (e.g. * alignment). It didn't get applied because Paolo disagreed with having our tools enforcing what our style guide says. Personally I think we should just commit my patch, and then we can stop having people manually pointing out where submitters' patches don't match CODING_STYLE. I am afraid that I can't give a firm option on this topic because it precedes my existence in this community, regardless I tend to agreed on Peter's reasoning. Thanks! - Wainer T thanks -- PMM
Re: [Qemu-devel] [PATCH 0/1] checkpatch: checker for comment block
On Fri, 14 Dec 2018 at 06:29, Markus Armbruster wrote: > > Paolo Bonzini writes: > > > On 13/12/18 19:21, Peter Maydell wrote: > >> On Thu, 13 Dec 2018 at 18:07, Paolo Bonzini wrote: > >>> On 13/12/18 19:01, Peter Maydell wrote: > I sent a patch to do this a little while back: > https://patchwork.kernel.org/patch/10561557/ > Personally I think we should just commit my patch, and then > we can stop having people manually pointing out where > submitters' patches don't match CODING_STYLE. > > Concur. It has my R-by, modulo a commit message tweak. I have to admit I never really understood what tweak you wanted making to the commit message. I'm happy to make it clearer: do you want to suggest a rewording? thanks -- PMM
Re: [Qemu-devel] [PATCH 0/1] checkpatch: checker for comment block
On 14/12/18 07:29, Markus Armbruster wrote:
> Paolo Bonzini writes:
>
>> On 13/12/18 19:21, Peter Maydell wrote:
>>> On Thu, 13 Dec 2018 at 18:07, Paolo Bonzini wrote:
On 13/12/18 19:01, Peter Maydell wrote:
> I sent a patch to do this a little while back:
> https://patchwork.kernel.org/patch/10561557/
>
> It didn't get applied because Paolo disagreed with having
> our tools enforcing what our style guide says.
I didn't disagree with that---I disagreed with having a single style in
the style guide, because unlike most other blatant violations of the
coding style (eg. braces), this one is pervasive in maintained code and
I don't want code that I maintain to mix two comment styles.
So I proposed two alternatives:
- someone fixes all the comment blocks which are "starred" but don't
have a lone "/*" at the beginning, and then we can commit that patch;
- we allow "/* foo" on the first line, except for doc comments and for
the first line of the file (author/license block), and fix the style
guide accordingly.
>>>
>>> We came to a consensus on the comment style when we discussed
>>> the patch which updated CODING_STYLE. I'm not personally
>>> a fan of the result (I used to use "/* foo"), but what we have in the
>>> doc is what we achieved consensus for. I'm not going to reopen
>>> the "what should block comments look like" style debate.
>>
>> Sure, I don't want to do that either. I accept the result of the
>> discussion; I don't accept introducing a new warning that will cause
>> over 700 files to become inconsistent sooner or later.
>
> By design, checkpatch.pl only checks *patches*. Existing code doesn't
> trigger warnings until it gets touched. And then it should arguably be
> made to conform to CODING_STYLE. So, what's the problem again? :)
Once you add multiline comments to a file that has 3-line comments, they
have to be 4-line in order to appease checkpatch, and this create
inconsistencies.
In other words, it's not about checkpatch complaining on existing code.
However, by running checkpatch on existing maintained code, you get a
measure of which files will grow an inconsistent style unless cleaned up
wholesale.
Anyway, in order to conclude the discussion and walk the walk, here is
the script. It also converts GNU style to the anointed one. I now
support applying Peter's patch, after the script is reviewed I'll send
it as a formal patch.
#! /bin/sh
#
# Fix multiline comments to match CODING_STYLE
#
# Copyright (C) 2018 Red Hat, Inc.
#
# Author: Paolo Bonzini
#
# Usage: scripts/fix-multiline-comments.sh [-i] FILE...
#
# -i edits the file in place (requires gawk 4.1.0).
#
# Set the AWK environment variable to choose the awk interpreter to use
# (default 'awk')
if test "$1" = -i; then
# gawk extension
inplace="-i inplace"
shift
fi
${AWK-awk} $inplace 'BEGIN {
getline first
indent = -1
while ((getline second) > 0) {
# apply a star to the indent on lines after the first
if (indent != -1) {
if (first == "") {
first = sp " *"
} else if (substr(first, 1, indent + 2) == sp " ") {
first = sp " *" substr(first, indent + 3)
}
}
is_lead = (first ~ /^[ \t]*\/\*/)
is_trail = (first ~ /\*\//)
if (is_lead && !is_trail) {
# grab the indent at the start of a comment, but not for
# single-line comments
match(first, /^[ \t]*\/\*/)
indent = RLENGTH - 2
sp = substr(first, 1, indent)
}
# the regular expression filters out lone /*, /**, or */
if (indent != -1 && !(first ~ /^[ \t]*(\/\*+|\*\/)[ \t]*$/)) {
if (is_trail) {
# split the trailing */ on a separate line
match(first, /[ \t]*\*\//)
first = substr(first, 1, RSTART - 1) "\n" sp " */"
}
if (is_lead) {
# split the leading /* on a separate line
match(first, /^[ \t]*\/\*+[ \t]*/)
first = sp "/*\n" sp " *" substr(first, RLENGTH)
}
}
if (is_trail) {
indent = -1
}
print first
first = second
}
print first
}' "$@"
Paolo
Re: [Qemu-devel] [PATCH 0/1] checkpatch: checker for comment block
Paolo Bonzini writes: > On 13/12/18 19:21, Peter Maydell wrote: >> On Thu, 13 Dec 2018 at 18:07, Paolo Bonzini wrote: >>> On 13/12/18 19:01, Peter Maydell wrote: I sent a patch to do this a little while back: https://patchwork.kernel.org/patch/10561557/ It didn't get applied because Paolo disagreed with having our tools enforcing what our style guide says. >>> >>> I didn't disagree with that---I disagreed with having a single style in >>> the style guide, because unlike most other blatant violations of the >>> coding style (eg. braces), this one is pervasive in maintained code and >>> I don't want code that I maintain to mix two comment styles. >>> >>> So I proposed two alternatives: >>> >>> - someone fixes all the comment blocks which are "starred" but don't >>> have a lone "/*" at the beginning, and then we can commit that patch; >>> >>> - we allow "/* foo" on the first line, except for doc comments and for >>> the first line of the file (author/license block), and fix the style >>> guide accordingly. >> >> We came to a consensus on the comment style when we discussed >> the patch which updated CODING_STYLE. I'm not personally >> a fan of the result (I used to use "/* foo"), but what we have in the >> doc is what we achieved consensus for. I'm not going to reopen >> the "what should block comments look like" style debate. > > Sure, I don't want to do that either. I accept the result of the > discussion; I don't accept introducing a new warning that will cause > over 700 files to become inconsistent sooner or later. By design, checkpatch.pl only checks *patches*. Existing code doesn't trigger warnings until it gets touched. And then it should arguably be made to conform to CODING_STYLE. So, what's the problem again? :) > Therefore, the > only way to enforce the result of the discussion is to change the > existing comments, I support cleaning up comment style wholesale[*]. >for example by having a script that maintainers can > use to change the existing comments in their files. Having each of us > come up with their own script or doing it by hand is probably not a good > use of everyone's time. Sharing tools is good. > Alternatively, fixing the style guide can also mean "explain why /* foo > is allowed by checkpatch even though it does not match the coding > style", without rehashing the discussion. > > (BTW it may actually be a good idea to fix _some_ instances of bad > coding style, in particular the space-tab sequences and the files where > there are maybe 2 or 3 tabs that ended up there by mistake. That's a > different topic). You've since posted patches for that. Thanks. Personally I think we should just commit my patch, and then we can stop having people manually pointing out where submitters' patches don't match CODING_STYLE. Concur. It has my R-by, modulo a commit message tweak. [*] Same for other style violations. Yes, it's churn, and yes, it'll mess up git-blame some, but I'm convinced the presence of numerous bad examples costs us more. CODING_STYLE was committed almost a decade ago. If we had cleaned up back then, the churn and the blame would be long forgotten, and we would've spared ourselves plenty of review cycles and quite a few style discussions. It's late, but never too late.
Re: [Qemu-devel] [PATCH 0/1] checkpatch: checker for comment block
On 13/12/18 19:21, Peter Maydell wrote: > On Thu, 13 Dec 2018 at 18:07, Paolo Bonzini wrote: >> On 13/12/18 19:01, Peter Maydell wrote: >>> I sent a patch to do this a little while back: >>> https://patchwork.kernel.org/patch/10561557/ >>> >>> It didn't get applied because Paolo disagreed with having >>> our tools enforcing what our style guide says. >> >> I didn't disagree with that---I disagreed with having a single style in >> the style guide, because unlike most other blatant violations of the >> coding style (eg. braces), this one is pervasive in maintained code and >> I don't want code that I maintain to mix two comment styles. >> >> So I proposed two alternatives: >> >> - someone fixes all the comment blocks which are "starred" but don't >> have a lone "/*" at the beginning, and then we can commit that patch; >> >> - we allow "/* foo" on the first line, except for doc comments and for >> the first line of the file (author/license block), and fix the style >> guide accordingly. > > We came to a consensus on the comment style when we discussed > the patch which updated CODING_STYLE. I'm not personally > a fan of the result (I used to use "/* foo"), but what we have in the > doc is what we achieved consensus for. I'm not going to reopen > the "what should block comments look like" style debate. Sure, I don't want to do that either. I accept the result of the discussion; I don't accept introducing a new warning that will cause over 700 files to become inconsistent sooner or later. Therefore, the only way to enforce the result of the discussion is to change the existing comments, for example by having a script that maintainers can use to change the existing comments in their files. Having each of us come up with their own script or doing it by hand is probably not a good use of everyone's time. Alternatively, fixing the style guide can also mean "explain why /* foo is allowed by checkpatch even though it does not match the coding style", without rehashing the discussion. (BTW it may actually be a good idea to fix _some_ instances of bad coding style, in particular the space-tab sequences and the files where there are maybe 2 or 3 tabs that ended up there by mistake. That's a different topic). Paolo
Re: [Qemu-devel] [PATCH 0/1] checkpatch: checker for comment block
On Thu, 13 Dec 2018 at 18:07, Paolo Bonzini wrote: > On 13/12/18 19:01, Peter Maydell wrote: > > I sent a patch to do this a little while back: > > https://patchwork.kernel.org/patch/10561557/ > > > > It didn't get applied because Paolo disagreed with having > > our tools enforcing what our style guide says. > > I didn't disagree with that---I disagreed with having a single style in > the style guide, because unlike most other blatant violations of the > coding style (eg. braces), this one is pervasive in maintained code and > I don't want code that I maintain to mix two comment styles. > > So I proposed two alternatives: > > - someone fixes all the comment blocks which are "starred" but don't > have a lone "/*" at the beginning, and then we can commit that patch; > > - we allow "/* foo" on the first line, except for doc comments and for > the first line of the file (author/license block), and fix the style > guide accordingly. We came to a consensus on the comment style when we discussed the patch which updated CODING_STYLE. I'm not personally a fan of the result (I used to use "/* foo"), but what we have in the doc is what we achieved consensus for. I'm not going to reopen the "what should block comments look like" style debate. We have always had older code which isn't following the new style, and our general approach is that we don't do mass-style-fix patches across the whole codebase. If you as a maintainer of a particular sub-area want to do a style update you're free to do that. thanks -- PMM
Re: [Qemu-devel] [PATCH 0/1] checkpatch: checker for comment block
On 13/12/18 19:01, Peter Maydell wrote: > On Thu, 13 Dec 2018 at 17:57, Wainer dos Santos Moschetta > wrote: >> >> Eduardo Habkost pointed out a malformed block of comments on my >> patch [1] that I had ran checkpatch.pl and no warn/error was >> reported. Then I realized the script does not catch such as >> case (or it had a bug). >> >> It turns out that checkpatch.pl does not parse comment blocks (If I >> understood >> its code correctly...). So I implemented a checker that warns about: >> 1. block doesn't begin on its own line. >> Example: >> /* blah blah >>* and blah blah >>*/ > > I sent a patch to do this a little while back: > https://patchwork.kernel.org/patch/10561557/ > > It didn't get applied because Paolo disagreed with having > our tools enforcing what our style guide says. I didn't disagree with that---I disagreed with having a single style in the style guide, because unlike most other blatant violations of the coding style (eg. braces), this one is pervasive in maintained code and I don't want code that I maintain to mix two comment styles. So I proposed two alternatives: - someone fixes all the comment blocks which are "starred" but don't have a lone "/*" at the beginning, and then we can commit that patch; - we allow "/* foo" on the first line, except for doc comments and for the first line of the file (author/license block), and fix the style guide accordingly. Paolo
Re: [Qemu-devel] [PATCH 0/1] checkpatch: checker for comment block
On Thu, 13 Dec 2018 at 17:57, Wainer dos Santos Moschetta wrote: > > Eduardo Habkost pointed out a malformed block of comments on my > patch [1] that I had ran checkpatch.pl and no warn/error was > reported. Then I realized the script does not catch such as > case (or it had a bug). > > It turns out that checkpatch.pl does not parse comment blocks (If I understood > its code correctly...). So I implemented a checker that warns about: > 1. block doesn't begin on its own line. > Example: > /* blah blah >* and blah blah >*/ I sent a patch to do this a little while back: https://patchwork.kernel.org/patch/10561557/ It didn't get applied because Paolo disagreed with having our tools enforcing what our style guide says. Personally I think we should just commit my patch, and then we can stop having people manually pointing out where submitters' patches don't match CODING_STYLE. thanks -- PMM
[Qemu-devel] [PATCH 0/1] checkpatch: checker for comment block
Eduardo Habkost pointed out a malformed block of comments on my patch [1] that I had ran checkpatch.pl and no warn/error was reported. Then I realized the script does not catch such as case (or it had a bug). It turns out that checkpatch.pl does not parse comment blocks (If I understood its code correctly...). So I implemented a checker that warns about: 1. block doesn't begin on its own line. Example: /* blah blah * and blah blah */ 2. line in the block doesn't start with asterisk. Example: /* foo bar bar foo */ Note: only the first occurence (i.e 'foo bar') is reported. 3. block doesn't end on its own line. Example: /* * blah blah * and blah */ References: [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg580091.html ps: last time I wrote Perl code was about 13 years ago. That remember me of good times. :) Wainer dos Santos Moschetta (1): checkpatch: check for malformed comment block. scripts/checkpatch.pl | 32 1 file changed, 32 insertions(+) -- 2.19.1
[Qemu-devel] [PATCH 0/1] Acceptance tests: add test for set-numa-node error handler fix
This cover letter does nothing but provides hopefully useful references to reviewers. More information on the patch itself. Git Info: - URI: https://github.com/clebergnu/qemu/tree/sent/set-numa-node - Remote: https://github.com/clebergnu/qemu - Branch: sent/set-numa-node Travis CI Info: - Build: https://travis-ci.org/clebergnu/qemu/builds/457721153 - Execution results for tests updated on this series: * https://travis-ci.org/clebergnu/qemu/jobs/457721176#L2051 * https://travis-ci.org/clebergnu/qemu/jobs/457721176#L2052 Cleber Rosa (1): Acceptance tests: add test for set-numa-node error handler fix tests/acceptance/set_numa_node.py | 41 +++ 1 file changed, 41 insertions(+) create mode 100644 tests/acceptance/set_numa_node.py -- 2.19.1
[Qemu-devel] [PATCH 0/1 V2] Add vhost-pci-blk driver
V2 changes: - checkpatch style fixes - correct size detection of disk image placed on a file system This driver moves virtio-blk host-side processing to kernel (via new vhost_blk kernel driver). It accelerates virtual disk performance close to the bare metal levels, especially for parellel loads. For example, fio numjobs=16 gets 101k randread IOPS using virtio-blk and 1202k IOPS using vhost-blk, close to 1480k of raw disk performance. See the IOPS numbers below. The kernel part if you want to try: - vhost_blk: https://lkml.org/lkml/2018/11/2/648 - vhost num-queues scalability fix: https://lkml.org/lkml/2018/11/2/550 # fio num-jobs # A: bare metal over block # B: bare metal over file # C: virtio-blk over block # D: virtio-blk over file # E: vhost-blk over block # F: vhost-blk over file # # A B CDE F 1 171k 151k 148k 151k 187k 175k 2 328k 302k 249k 241k 334k 296k 3 479k 437k 179k 174k 464k 404k 4 622k 568k 143k 183k 580k 492k 5 755k 697k 136k 128k 693k 579k 6 887k 808k 131k 120k 782k 640k 7 1004k 926k 126k 131k 863k 693k 8 1099k 1015k 117k 115k 931k 712k 9 1194k 1119k 115k 111k 991k 711k 10 1278k 1207k 109k 114k 1046k 695k 11 1345k 1280k 110k 108k 1091k 663k 12 1411k 1356k 104k 106k 1142k 629k 13 1466k 1423k 106k 106k 1170k 607k 14 1517k 1486k 103k 106k 1179k 589k 15 1552k 1543k 102k 102k 1191k 571k 16 1480k 1506k 101k 102k 1202k 566k Vitaly Mayatskikh (1): Add vhost-pci-blk driver configure | 10 + default-configs/virtio.mak| 1 + hw/block/Makefile.objs| 1 + hw/block/vhost-blk.c | 429 ++ hw/virtio/virtio-pci.c| 60 + hw/virtio/virtio-pci.h| 19 ++ include/hw/virtio/vhost-blk.h | 43 7 files changed, 563 insertions(+) create mode 100644 hw/block/vhost-blk.c create mode 100644 include/hw/virtio/vhost-blk.h -- 2.17.1
Re: [Qemu-devel] [PATCH 0/1 resend] Add vhost-pci-blk driver
On Mon, Nov 5, 2018 at 12:45 PM Michael S. Tsirkin wrote: > I think you should Cc more widely to get meaningful > review. At least virtio-blk and block layer core people. Thanks, it turns out I missed the existence of qemu/scripts directory completely. -- wbr, Vitaly
Re: [Qemu-devel] [PATCH 0/1 resend] Add vhost-pci-blk driver
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20181105140327.8363-1-v.mayats...@gmail.com
Subject: [Qemu-devel] [PATCH 0/1 resend] Add vhost-pci-blk driver
=== 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'
19803cc4ae Add vhost-pci-blk driver
=== OUTPUT BEGIN ===
Checking PATCH 1/1: Add vhost-pci-blk driver...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#82:
new file mode 100644
ERROR: Error messages should not contain newlines
#229: FILE: hw/block/vhost-blk.c:143:
+error_report("Error opening backing store: %d\n", -errno);
ERROR: braces {} are necessary for all arms of this statement
#241: FILE: hw/block/vhost-blk.c:155:
+if (s->bs_fd > 0)
[...]
ERROR: space prohibited after that '-' (ctx:WxW)
#327: FILE: hw/block/vhost-blk.c:241:
+int fd = - 1;
^
WARNING: line over 80 characters
#331: FILE: hw/block/vhost-blk.c:245:
+ error_report("Can't open device %s: %d\n",
blk_bs(s->blk)->filename, errno);
ERROR: code indent should never use tabs
#331: FILE: hw/block/vhost-blk.c:245:
+^Ierror_report("Can't open device %s: %d\n", blk_bs(s->blk)->filename,
errno);$
ERROR: Error messages should not contain newlines
#331: FILE: hw/block/vhost-blk.c:245:
+ error_report("Can't open device %s: %d\n",
blk_bs(s->blk)->filename, errno);
ERROR: code indent should never use tabs
#332: FILE: hw/block/vhost-blk.c:246:
+^Igoto out;$
ERROR: code indent should never use tabs
#336: FILE: hw/block/vhost-blk.c:250:
+^Iret = ioctl(fd, BLKGETSIZE, &var);$
ERROR: code indent should never use tabs
#337: FILE: hw/block/vhost-blk.c:251:
+^Ivar64 = var;$
ERROR: code indent should never use tabs
#340: FILE: hw/block/vhost-blk.c:254:
+^Ierror_report("Can't get drive size: %d\n", errno);$
ERROR: Error messages should not contain newlines
#340: FILE: hw/block/vhost-blk.c:254:
+ error_report("Can't get drive size: %d\n", errno);
ERROR: code indent should never use tabs
#341: FILE: hw/block/vhost-blk.c:255:
+^Igoto out;$
ERROR: line over 90 characters
#347: FILE: hw/block/vhost-blk.c:261:
+ error_report("Can't get drive logical sector size, assuming 512:
%d\n", errno);
ERROR: code indent should never use tabs
#347: FILE: hw/block/vhost-blk.c:261:
+^Ierror_report("Can't get drive logical sector size, assuming 512: %d\n",
errno);$
ERROR: Error messages should not contain newlines
#347: FILE: hw/block/vhost-blk.c:261:
+ error_report("Can't get drive logical sector size, assuming 512:
%d\n", errno);
ERROR: code indent should never use tabs
#348: FILE: hw/block/vhost-blk.c:262:
+^Ivar = 512;$
ERROR: braces {} are necessary for all arms of this statement
#360: FILE: hw/block/vhost-blk.c:274:
+if (fd > 0)
[...]
ERROR: code indent should never use tabs
#361: FILE: hw/block/vhost-blk.c:275:
+^Iclose(fd);$
ERROR: code indent should never use tabs
#410: FILE: hw/block/vhost-blk.c:324:
+^Igoto virtio_err;$
ERROR: line over 90 characters
#413: FILE: hw/block/vhost-blk.c:327:
+ret = vhost_dev_init(&s->dev, (void *)(uintptr_t)s->vhostfd,
VHOST_BACKEND_TYPE_KERNEL, 0);
total: 19 errors, 2 warnings, 613 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
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com
Re: [Qemu-devel] [PATCH 0/1 resend] Add vhost-pci-blk driver
On Mon, Nov 05, 2018 at 02:03:26PM +, Vitaly Mayatskikh wrote: > This driver moves virtio-blk host-side processing to kernel (via new > vhost_blk kernel driver). It accelerates virtual disk performance > close to bare metal levels, especially for parellel loads. > > For example, fio numjobs=16 gets 101k randread IOPS using virtio-blk > and 1202k IOPS using vhost-blk, close to 1480k of raw disk performance. > > See the IOPS numbers below. > > The kernel part if you want to try: > - vhost_blk: https://lkml.org/lkml/2018/11/2/648 > - vhost num-queues scalability fix: https://lkml.org/lkml/2018/11/2/550 > > # fio num-jobs > # A: bare metal over block > # B: bare metal over file > # C: virtio-blk over block > # D: virtio-blk over file > # E: vhost-blk over block > # F: vhost-blk over file > # > # A B CDE F > > 1 171k 151k 148k 151k 187k 175k > 2 328k 302k 249k 241k 334k 296k > 3 479k 437k 179k 174k 464k 404k > 4 622k 568k 143k 183k 580k 492k > 5 755k 697k 136k 128k 693k 579k > 6 887k 808k 131k 120k 782k 640k > 7 1004k 926k 126k 131k 863k 693k > 8 1099k 1015k 117k 115k 931k 712k > 9 1194k 1119k 115k 111k 991k 711k > 10 1278k 1207k 109k 114k 1046k 695k > 11 1345k 1280k 110k 108k 1091k 663k > 12 1411k 1356k 104k 106k 1142k 629k > 13 1466k 1423k 106k 106k 1170k 607k > 14 1517k 1486k 103k 106k 1179k 589k > 15 1552k 1543k 102k 102k 1191k 571k > 16 1480k 1506k 101k 102k 1202k 566k > > Vitaly Mayatskikh (1): > Add vhost-pci-blk driver > > configure | 10 +++ > default-configs/virtio.mak | 1 + > hw/block/Makefile.objs | 1 + > hw/virtio/virtio-pci.c | 60 ++ > hw/virtio/virtio-pci.h | 19 > 5 files changed, 91 insertions(+) I think you should Cc more widely to get meaningful review. At least virtio-blk and block layer core people. > -- > 2.17.1
[Qemu-devel] [PATCH 0/1 resend] Add vhost-pci-blk driver
This driver moves virtio-blk host-side processing to kernel (via new vhost_blk kernel driver). It accelerates virtual disk performance close to bare metal levels, especially for parellel loads. For example, fio numjobs=16 gets 101k randread IOPS using virtio-blk and 1202k IOPS using vhost-blk, close to 1480k of raw disk performance. See the IOPS numbers below. The kernel part if you want to try: - vhost_blk: https://lkml.org/lkml/2018/11/2/648 - vhost num-queues scalability fix: https://lkml.org/lkml/2018/11/2/550 # fio num-jobs # A: bare metal over block # B: bare metal over file # C: virtio-blk over block # D: virtio-blk over file # E: vhost-blk over block # F: vhost-blk over file # # A B CDE F 1 171k 151k 148k 151k 187k 175k 2 328k 302k 249k 241k 334k 296k 3 479k 437k 179k 174k 464k 404k 4 622k 568k 143k 183k 580k 492k 5 755k 697k 136k 128k 693k 579k 6 887k 808k 131k 120k 782k 640k 7 1004k 926k 126k 131k 863k 693k 8 1099k 1015k 117k 115k 931k 712k 9 1194k 1119k 115k 111k 991k 711k 10 1278k 1207k 109k 114k 1046k 695k 11 1345k 1280k 110k 108k 1091k 663k 12 1411k 1356k 104k 106k 1142k 629k 13 1466k 1423k 106k 106k 1170k 607k 14 1517k 1486k 103k 106k 1179k 589k 15 1552k 1543k 102k 102k 1191k 571k 16 1480k 1506k 101k 102k 1202k 566k Vitaly Mayatskikh (1): Add vhost-pci-blk driver configure | 10 +++ default-configs/virtio.mak | 1 + hw/block/Makefile.objs | 1 + hw/virtio/virtio-pci.c | 60 ++ hw/virtio/virtio-pci.h | 19 5 files changed, 91 insertions(+) -- 2.17.1
[Qemu-devel] [PATCH 0/1] Add vhost-pci-blk driver
This driver moves virtio-blk host-side processing to kernel (via new vhost_blk kernel driver). It accelerates virtual disk performance close to bare metal levels, especially for parellel loads. For example, fio numjobs=16 gets 101k randread IOPS using virtio-blk and 1202k IOPS using vhost-blk, close to 1480k of raw disk performance. See the IOPS numbers below. The kernel part if you want to try: - vhost_blk: https://lkml.org/lkml/2018/11/2/648 - vhost num-queues scalability fix: https://lkml.org/lkml/2018/11/2/550 # fio num-jobs # A: bare metal over block # B: bare metal over file # C: virtio-blk over block # D: virtio-blk over file # E: vhost-blk over block # F: vhost-blk over file # # A B CDE F 1 171k 151k 148k 151k 187k 175k 2 328k 302k 249k 241k 334k 296k 3 479k 437k 179k 174k 464k 404k 4 622k 568k 143k 183k 580k 492k 5 755k 697k 136k 128k 693k 579k 6 887k 808k 131k 120k 782k 640k 7 1004k 926k 126k 131k 863k 693k 8 1099k 1015k 117k 115k 931k 712k 9 1194k 1119k 115k 111k 991k 711k 10 1278k 1207k 109k 114k 1046k 695k 11 1345k 1280k 110k 108k 1091k 663k 12 1411k 1356k 104k 106k 1142k 629k 13 1466k 1423k 106k 106k 1170k 607k 14 1517k 1486k 103k 106k 1179k 589k 15 1552k 1543k 102k 102k 1191k 571k 16 1480k 1506k 101k 102k 1202k 566k Vitaly Mayatskikh (1): Add vhost-pci-blk driver configure | 10 +++ default-configs/virtio.mak | 1 + hw/block/Makefile.objs | 1 + hw/virtio/virtio-pci.c | 60 ++ hw/virtio/virtio-pci.h | 19 5 files changed, 91 insertions(+) -- 2.17.1
[Qemu-devel] [PATCH 0/1] Add PKU/OSPKE on Skylake-Server CPU model
This patch adds PKU/OSPKE on Skylake-Server CPU model Tao Xu (1): i386: Add PKU/OSPKE on Skylake-Server CPU model target/i386/cpu.c | 4 1 file changed, 4 insertions(+) -- 2.17.1
Re: [Qemu-devel] [PATCH 0/1] add exit-script option to qemu
On Fri, Oct 05, 2018 at 02:57:03PM +0200, Dominik Csapak wrote: > On 10/5/18 10:38 AM, Daniel P. Berrangé wrote: > > On Fri, Oct 05, 2018 at 08:56:27AM +0200, Dominik Csapak wrote: > > > On 10/4/18 3:51 PM, Daniel P. Berrangé wrote: > > > > On Wed, Oct 03, 2018 at 11:13:43AM +0200, Dominik Csapak wrote: > > > > > this patch aims to execute a script when qemu exits > > > > > so that one can do cleanups when using --daemonize without > > > > > having to use the qmp monitor > > > > > > > > IMHO the idea of cleanup scripts run by QEMU itself is flawed. > > > > QEMU will inevitably crash before cleanup scripts can be run, > > > > so whatever mgmt app is using QEMU needs to be able to do > > > > cleanup without QEMU's help. > > > > > > > > I think this can be done more reliably with a wrapper script, > > > > that spawns QEMU, waits for it to exit and then calls the > > > > cleanup script. On Linux at least you can use prctl() with > > > > PR_SET_CHILD_SUBREAPER so you can detect exit'ing of QEMU > > > > even after it has daemonized. > > > > > > > > Perhaps we could have such a wrapper script put in the > > > > contrib directory > > > > > > > > Regards, > > > > Daniel > > > > > > > Hi, > > > > > > for cleaning up after qemu crashes, you are completely right, > > > (ignoring that the downscript for tap devices also never gets executed > > > then), but this series has another use. > > > > > > With it, a user can determine the reason of a graceful shutdown > > > (e.g., if it was by a signal, qmp or from inside) of qemu, > > > especially when using -no-reboot without using qmp > > > > > > and using qmp for that is not very practical for everyone, > > > or is there another way for that which i am missing? > > > > Honestly QMP *is* the right answer. We've put alot of effort into QMP > > and I don't think it is sensible to start adding new mechanisms to > > provide the same information in an adhoc manner. > > > > What makes you think QMP isn't practical to use ? We have client > > impls that talk to QMP in scripts/qmp that are just a few 100 lines > > of pretty simple python code. > > > > ok, i just found that having to start an extra program waiting for qmp > events might be overkill for some users > > nonetheless, i just found out that even with qmp, there is no way > to see if a machine started with '-no-reboot' was trying to reboot > or just shutting down, in both cases i got a SHUTDOWN event > with 'guest: true' > > would it make sense to send a patch that introduces a new data field > for the shutdown event that says if it was really a reset? I had a feeling there was another way to detct it, but I'm not seeing it in QMP. So yeah, if this can't be determined, then I expect it is worth extending QMP to report this 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] [PATCH 0/1] add exit-script option to qemu
On 10/5/18 10:38 AM, Daniel P. Berrangé wrote: On Fri, Oct 05, 2018 at 08:56:27AM +0200, Dominik Csapak wrote: On 10/4/18 3:51 PM, Daniel P. Berrangé wrote: On Wed, Oct 03, 2018 at 11:13:43AM +0200, Dominik Csapak wrote: this patch aims to execute a script when qemu exits so that one can do cleanups when using --daemonize without having to use the qmp monitor IMHO the idea of cleanup scripts run by QEMU itself is flawed. QEMU will inevitably crash before cleanup scripts can be run, so whatever mgmt app is using QEMU needs to be able to do cleanup without QEMU's help. I think this can be done more reliably with a wrapper script, that spawns QEMU, waits for it to exit and then calls the cleanup script. On Linux at least you can use prctl() with PR_SET_CHILD_SUBREAPER so you can detect exit'ing of QEMU even after it has daemonized. Perhaps we could have such a wrapper script put in the contrib directory Regards, Daniel Hi, for cleaning up after qemu crashes, you are completely right, (ignoring that the downscript for tap devices also never gets executed then), but this series has another use. With it, a user can determine the reason of a graceful shutdown (e.g., if it was by a signal, qmp or from inside) of qemu, especially when using -no-reboot without using qmp and using qmp for that is not very practical for everyone, or is there another way for that which i am missing? Honestly QMP *is* the right answer. We've put alot of effort into QMP and I don't think it is sensible to start adding new mechanisms to provide the same information in an adhoc manner. What makes you think QMP isn't practical to use ? We have client impls that talk to QMP in scripts/qmp that are just a few 100 lines of pretty simple python code. ok, i just found that having to start an extra program waiting for qmp events might be overkill for some users nonetheless, i just found out that even with qmp, there is no way to see if a machine started with '-no-reboot' was trying to reboot or just shutting down, in both cases i got a SHUTDOWN event with 'guest: true' would it make sense to send a patch that introduces a new data field for the shutdown event that says if it was really a reset?
Re: [Qemu-devel] [PATCH 0/1] add exit-script option to qemu
On Fri, Oct 05, 2018 at 08:56:27AM +0200, Dominik Csapak wrote: > On 10/4/18 3:51 PM, Daniel P. Berrangé wrote: > > On Wed, Oct 03, 2018 at 11:13:43AM +0200, Dominik Csapak wrote: > > > this patch aims to execute a script when qemu exits > > > so that one can do cleanups when using --daemonize without > > > having to use the qmp monitor > > > > IMHO the idea of cleanup scripts run by QEMU itself is flawed. > > QEMU will inevitably crash before cleanup scripts can be run, > > so whatever mgmt app is using QEMU needs to be able to do > > cleanup without QEMU's help. > > > > I think this can be done more reliably with a wrapper script, > > that spawns QEMU, waits for it to exit and then calls the > > cleanup script. On Linux at least you can use prctl() with > > PR_SET_CHILD_SUBREAPER so you can detect exit'ing of QEMU > > even after it has daemonized. > > > > Perhaps we could have such a wrapper script put in the > > contrib directory > > > > Regards, > > Daniel > > > Hi, > > for cleaning up after qemu crashes, you are completely right, > (ignoring that the downscript for tap devices also never gets executed > then), but this series has another use. > > With it, a user can determine the reason of a graceful shutdown > (e.g., if it was by a signal, qmp or from inside) of qemu, > especially when using -no-reboot without using qmp > > and using qmp for that is not very practical for everyone, > or is there another way for that which i am missing? Honestly QMP *is* the right answer. We've put alot of effort into QMP and I don't think it is sensible to start adding new mechanisms to provide the same information in an adhoc manner. What makes you think QMP isn't practical to use ? We have client impls that talk to QMP in scripts/qmp that are just a few 100 lines of pretty simple python code. 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] [PATCH 0/1] add exit-script option to qemu
On 10/4/18 3:51 PM, Daniel P. Berrangé wrote: On Wed, Oct 03, 2018 at 11:13:43AM +0200, Dominik Csapak wrote: this patch aims to execute a script when qemu exits so that one can do cleanups when using --daemonize without having to use the qmp monitor IMHO the idea of cleanup scripts run by QEMU itself is flawed. QEMU will inevitably crash before cleanup scripts can be run, so whatever mgmt app is using QEMU needs to be able to do cleanup without QEMU's help. I think this can be done more reliably with a wrapper script, that spawns QEMU, waits for it to exit and then calls the cleanup script. On Linux at least you can use prctl() with PR_SET_CHILD_SUBREAPER so you can detect exit'ing of QEMU even after it has daemonized. Perhaps we could have such a wrapper script put in the contrib directory Regards, Daniel Hi, for cleaning up after qemu crashes, you are completely right, (ignoring that the downscript for tap devices also never gets executed then), but this series has another use. With it, a user can determine the reason of a graceful shutdown (e.g., if it was by a signal, qmp or from inside) of qemu, especially when using -no-reboot without using qmp and using qmp for that is not very practical for everyone, or is there another way for that which i am missing? Regards, Dominik
Re: [Qemu-devel] [PATCH 0/1] add exit-script option to qemu
On Wed, Oct 03, 2018 at 11:13:43AM +0200, Dominik Csapak wrote: > this patch aims to execute a script when qemu exits > so that one can do cleanups when using --daemonize without > having to use the qmp monitor IMHO the idea of cleanup scripts run by QEMU itself is flawed. QEMU will inevitably crash before cleanup scripts can be run, so whatever mgmt app is using QEMU needs to be able to do cleanup without QEMU's help. I think this can be done more reliably with a wrapper script, that spawns QEMU, waits for it to exit and then calls the cleanup script. On Linux at least you can use prctl() with PR_SET_CHILD_SUBREAPER so you can detect exit'ing of QEMU even after it has daemonized. Perhaps we could have such a wrapper script put in the contrib directory 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] [PATCH 0/1] add exit-script option to qemu
Hi Dominik, On 03/10/2018 11:13, Dominik Csapak wrote: > this patch aims to execute a script when qemu exits > so that one can do cleanups when using --daemonize without > having to use the qmp monitor > > for now i have mostly copied the script execution code from > the launch_script function of net/tap.c as i am not sure > if it would make sense to refactor that and if it does, where > to put such a function I'd rather refactor a 'more of 10 common LOC function'. Maybe qemu_launch_script() in "qemu/osdep.h"? Phil.
[Qemu-devel] [PATCH 0/1] add exit-script option to qemu
this patch aims to execute a script when qemu exits so that one can do cleanups when using --daemonize without having to use the qmp monitor for now i have mostly copied the script execution code from the launch_script function of net/tap.c as i am not sure if it would make sense to refactor that and if it does, where to put such a function Dominik Csapak (1): vl.c: call optional script when exiting qemu-options.hx | 18 ++ vl.c| 45 + 2 files changed, 63 insertions(+) -- 2.11.0
[Qemu-devel] [PATCH 0/1] Add new model of Cascadelake-Server
This patch defines the new guest CPU models of Cascadelake-Server. Tao Xu (1): i386: Add new model of Cascadelake-Server target/i386/cpu.c | 54 +++ 1 file changed, 54 insertions(+) -- 2.17.1
[Qemu-devel] [PATCH 0/1] libqtest: Improve error reporting for bad read from QEMU
Eric Blake's "[PATCH v4] tests/libqtest: Improve kill_qemu()" improves how we report how QEMU terminated. Sadly, we bypass kill_qemu() in certain failure modes. Fix that. Based-on: <20180810132800.38549-1-ebl...@redhat.com> Markus Armbruster (1): libqtest: Improve error reporting for bad read from QEMU tests/libqtest.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) -- 2.17.1
Re: [Qemu-devel] [PATCH 0/1] target/m68k: correctly disassemble move16
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20180625203559.21370-1-laur...@vivier.eu
Subject: [Qemu-devel] [PATCH 0/1] target/m68k: correctly disassemble move16
=== 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'
ce9f09fe63 target/m68k: correctly disassemble move16
=== OUTPUT BEGIN ===
Checking PATCH 1/1: target/m68k: correctly disassemble move16...
ERROR: that open brace { should be on the previous line
#30: FILE: disas/m68k.c:2021:
+ if (*d == '\0')
+{
ERROR: suspect code indent for conditional statements (10, 14)
#30: FILE: disas/m68k.c:2021:
+ if (*d == '\0')
+{
ERROR: that open brace { should be on the previous line
#32: FILE: disas/m68k.c:2023:
+ for (d = opc->args; *d; d += 2)
+{
ERROR: suspect code indent for conditional statements (14, 18)
#32: FILE: disas/m68k.c:2023:
+ for (d = opc->args; *d; d += 2)
+{
ERROR: that open brace { should be on the previous line
#34: FILE: disas/m68k.c:2025:
+ if (d[0] == 'I')
+{
ERROR: suspect code indent for conditional statements (18, 22)
#34: FILE: disas/m68k.c:2025:
+ if (d[0] == 'I')
+{
ERROR: space prohibited between function name and open parenthesis '('
#36: FILE: disas/m68k.c:2027:
+ val = fetch_arg (buffer, 'd', 3, info);
ERROR: braces {} are necessary for all arms of this statement
#37: FILE: disas/m68k.c:2028:
+ if (val != 1)
[...]
total: 8 errors, 0 warnings, 20 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
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com
[Qemu-devel] [PATCH 0/1] target/m68k: correctly disassemble move16
"move16 %a0@+,%a1@" and "fmovel (cpid=3) %a0@-,%fpcr" share the same opcode... but QEMU executes move16 (and M68040 too). You can try: --8<--- move16.S .data src: .long 0x01020304, 0x05060708, 0x090a0b0c, 0x0d0e0f00 dst: .long 0, 0, 0, 0 .text .globl _start _start: lea src,%a0 lea dst,%a1 .fopt id=3 fmovel -(%a0),%fpcr move16 (%a0)+, (%a1)+ move.l #0,%d1 move.l #1, %d0 trap #0 --8<--- move16.S m68k-linux-gnu-gcc -g -m68040 -nostartfiles -nodefaultlibs \ -nostdlib -o move16 move16.S m68k-linux-gnu-objdump -d move16 move16: file format elf32-m68k Disassembly of section .text: 80b8 <_start>: 80b8: 41f9 8000 20d4 lea 800020d4 ,%a0 80be: 43f9 8000 20e4 lea 800020e4 ,%a1 80c4: f620 9000 move16 %a0@+,%a1@+ 80c8: f620 9000 move16 %a0@+,%a1@+ 80cc: 7200moveq #0,%d1 80ce: 7001moveq #1,%d0 80d0: 4e40trap #0 qemu-m68k -d in_asm ./move16 IN: 0x80b8: lea 0x800020d4,%a0 0x80be: lea 0x800020e4,%a1 0x80c4: fmovel (cpid=3) %a0@-,%fpcr 0x80c8: fmovel (cpid=3) %a0@-,%fpcr 0x80cc: moveq #0,%d1 0x80ce: moveq #1,%d0 0x80d0: trap #0 This patch backports the fix from binutils to only match FPU instructions with coprocessor ID 1. Laurent Vivier (1): target/m68k: correctly disassemble move16 disas/m68k.c | 14 ++ 1 file changed, 14 insertions(+) -- 2.14.4
Re: [Qemu-devel] [PATCH 0/1] blockdev: implement n-ary bitmap merge
On 06/11/2018 06:43 PM, John Snow wrote: > requires: 20180606182449.1607-1-js...@redhat.com No longer requires any prerequisites. --js > > See patch for details; this is somewhat an RFC that I suspect > will be useful for libvirt in some situations, but maybe it's > actually overkill. > > John Snow (1): > blockdev: n-ary bitmap merge > > blockdev.c | 40 ++-- > qapi/block-core.json | 10 +- > 2 files changed, 35 insertions(+), 15 deletions(-) >
Re: [Qemu-devel] [PATCH 0/1] blockdev: implement n-ary bitmap merge
I mis-typed Vladimir's email, I have corrected it here. On 06/11/2018 06:43 PM, John Snow wrote: > requires: 20180606182449.1607-1-js...@redhat.com > > See patch for details; this is somewhat an RFC that I suspect > will be useful for libvirt in some situations, but maybe it's > actually overkill. > > John Snow (1): > blockdev: n-ary bitmap merge > > blockdev.c | 40 ++-- > qapi/block-core.json | 10 +- > 2 files changed, 35 insertions(+), 15 deletions(-) >
[Qemu-devel] [PATCH 0/1] blockdev: implement n-ary bitmap merge
requires: 20180606182449.1607-1-js...@redhat.com See patch for details; this is somewhat an RFC that I suspect will be useful for libvirt in some situations, but maybe it's actually overkill. John Snow (1): blockdev: n-ary bitmap merge blockdev.c | 40 ++-- qapi/block-core.json | 10 +- 2 files changed, 35 insertions(+), 15 deletions(-) -- 2.14.3
