Re: [PATCH v5 0/4] introduction of migration_version attribute for VFIO live migration
[meta-comment] On 4/29/20 4:35 AM, Yan Zhao wrote: On Wed, Apr 29, 2020 at 04:22:01PM +0800, Dr. David Alan Gilbert wrote: [...] This patchset introduces a migration_version attribute under sysfs of VFIO Mediated devices. Hmm, several pages with up to 16 levels of quoting, with editors making the lines ragged, all before I get to the real meat of the email. Remember, it's okay to trim content,... So why don't we split the difference; lets say that it should start with the hex PCI Vendor ID. The problem is for mdev devices, if the parent devices are not PCI devices, they don't have PCI vendor IDs. ...to just what you are replying to. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 2/2] nbd: add support for nbd as root device
On 6/13/19 8:02 AM, Eric Blake wrote: > On 6/12/19 11:31 AM, roman.stratiie...@globallogic.com wrote: >> From: Roman Stratiienko >> >> Adding support to nbd to use it as a root device. This code essentially >> provides a minimal nbd-client implementation within the kernel. It opens >> a socket and makes the negotiation with the server. Afterwards it passes >> the socket to the normal nbd-code to handle the connection. >> >> The arguments for the server are passed via kernel command line. >> The kernel command line has the format >> 'nbdroot=[:]/'. > > Did you intend for nbdroot=1234 to connect to port 1234 or to server > 1234 port 10809? Is an export name mandatory even when it is the empty > string, in which case, is the / character mandatory? Maybe this would > be better written as: > > [[:][:]][/[]] as well as a blurb that IPv6 requires use of [] around SERVER_IP to avoid ambiguity between IP address and the port designator. That would allow variations such as: nbdroot=1.2.3.4 # port 10809 and export name '' defaulted nbdroot=1.2.3.4:10809/ # fully explicit, export name '' nbdroot=:10810/export# host defaulted by DHCP, rest is explicit nbdroot=/# host and port are defaulted, export is '' nbdroot=[::1]:10810 # IPv6 localhost and port 10810 nbdroot=::1 # IPv6 localhost, default port 10809 [I'm aware that localhost is probably not going to work based on how early this is encountered during the boot; rather, consider ::1 as a placeholder for a more realistic IPv6 address] > > although that would allow nbdroot= using all defaults (will that still > do the right thing?). nbdroot=# host from DHCP, port 10809, export '' > > Should we support nbdroot=URI, and tie this in to Rich's proposal [1] on > standardizing the set of URIs that refer to an NBD export? It seems > like you are still limited to a TCP socket (not Unix) with no > encryption, so this would be equivalent to the URI: > > nbd://[server[:port]][/export] > > [1] https://lists.debian.org/nbd/2019/06/msg00011.html > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [PATCH 2/2] nbd: add support for nbd as root device
On 6/13/19 9:45 AM, Roman Stratiienko wrote: >> >> Just throw nbd-client in your initramfs. Every nbd server has it's own >> handshake protocol, embedding one particular servers handshake protocol into >> the >> kernel isn't the answer here. Thanks, The handshake protocol is well-specified: https://github.com/NetworkBlockDevice/nbd/blob/cdb0bc57f3faefd7a5562d57ad57cd990781c415/doc/proto.md All servers implement various subsets of that document for the handshake. > Also, as far as I know mainline nbd-server daemon have only 2 > handshake protocols. So called OLD-STYLE and NEW-STYLE. And OLD-STYLE > is no longer supported. So it should not be a problem, or please fix > me if I'm wrong. You are correct that oldstyle is no longer recommended. However, the current NBD specification states that newstyle has two different flavors, NBD_OPT_EXPORT_NAME (which you used, but is also old) and NBD_OPT_GO (which is newer, but is more likely to encounter differences where not all servers support it). The NBD specification includes a compatibility baseline: https://github.com/NetworkBlockDevice/nbd/blob/cdb0bc57f3faefd7a5562d57ad57cd990781c415/doc/proto.md#compatibility-and-interoperability and right now, NBD_OPT_GO (and _not_ NBD_OPT_EXPORT_NAME) is the preferred way forward. As long as your handshake implementation complies with the baseline documented there, you'll have maximum portability to the largest number of servers that also support the baseline - but not all servers are up to that baseline yet. So, this becomes a question of how much are you reinventing baseline portability handshake concerns in the kernel, vs. in initramfs. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [RFC v3] qemu: Add virtio pmem device
On 07/25/2018 12:01 AM, Pankaj Gupta wrote: This patch has no n/M in the subject line; but is included in a thread that also has a 0/2 cover letter, as well as 1/2 and 2/2 patches in separate mails. Is that intentional? Yes, kernel series has 0-2 patches and Qemu has this one. I thought its good to keep separate numbering for both the sets. Ah, that makes sense. The cover letter didn't make it obvious to me that this was two separate series to two projects, but related enough that both series have to be incorporated for the feature to work and thus cross-posted under a single cover letter. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [RFC v3] qemu: Add virtio pmem device
On 07/25/2018 12:01 AM, Pankaj Gupta wrote: This patch has no n/M in the subject line; but is included in a thread that also has a 0/2 cover letter, as well as 1/2 and 2/2 patches in separate mails. Is that intentional? Yes, kernel series has 0-2 patches and Qemu has this one. I thought its good to keep separate numbering for both the sets. Ah, that makes sense. The cover letter didn't make it obvious to me that this was two separate series to two projects, but related enough that both series have to be incorporated for the feature to work and thus cross-posted under a single cover letter. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [RFC v2] qemu: Add virtio pmem device
On 04/25/2018 06:24 AM, Pankaj Gupta wrote:
> This patch adds virtio-pmem Qemu device.
>
> This device presents memory address range
> information to guest which is backed by file
> backend type. It acts like persistent memory
> device for KVM guest. Guest can perform read
> and persistent write operations on this memory
> range with the help of DAX capable filesystem.
>
> Persistent guest writes are assured with the
> help of virtio based flushing interface. When
> guest userspace space performs fsync on file
> fd on pmem device, a flush command is send to
> Qemu over VIRTIO and host side flush/sync is
> done on backing image file.
>
> This PV device code is dependent and tested
> with 'David Hildenbrand's ' patchset[1] to
> map non-PCDIMM devices to guest address space.
This sentence doesn't belong in git history. It is better to put
information like this...
> There is still upstream discussion on using
> among PCI bar vs memory device, will update
> as per concensus.
s/concensus/consensus/
>
> [1] https://marc.info/?l=qemu-devel=152450249319168=2
>
> Signed-off-by: Pankaj Gupta <pagu...@redhat.com>
> ---
...here, where it is part of the email, but not picked up by 'git am'.
> +++ b/qapi/misc.json
> @@ -2871,6 +2871,29 @@
>}
> }
>
> +##
> +# @VirtioPMemDeviceInfo:
> +#
> +# VirtioPMem state information
> +#
> +# @id: device's ID
> +#
> +# @start: physical address, where device is mapped
> +#
> +# @size: size of memory that the device provides
> +#
> +# @memdev: memory backend linked with device
> +#
> +# Since: 2.13
> +##
> +{ 'struct': 'VirtioPMemDeviceInfo',
> +'data': { '*id': 'str',
> + 'start': 'size',
> + 'size': 'size',
TAB damage.
> + 'memdev': 'str'
> + }
> +}
> +
> ##
> # @MemoryDeviceInfo:
> #
> @@ -2880,7 +2903,8 @@
> ##
> { 'union': 'MemoryDeviceInfo',
>'data': { 'dimm': 'PCDIMMDeviceInfo',
> -'nvdimm': 'PCDIMMDeviceInfo'
> +'nvdimm': 'PCDIMMDeviceInfo',
> + 'virtio-pmem': 'VirtioPMemDeviceInfo'
>}
> }
>
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [RFC v2] qemu: Add virtio pmem device
On 04/25/2018 06:24 AM, Pankaj Gupta wrote:
> This patch adds virtio-pmem Qemu device.
>
> This device presents memory address range
> information to guest which is backed by file
> backend type. It acts like persistent memory
> device for KVM guest. Guest can perform read
> and persistent write operations on this memory
> range with the help of DAX capable filesystem.
>
> Persistent guest writes are assured with the
> help of virtio based flushing interface. When
> guest userspace space performs fsync on file
> fd on pmem device, a flush command is send to
> Qemu over VIRTIO and host side flush/sync is
> done on backing image file.
>
> This PV device code is dependent and tested
> with 'David Hildenbrand's ' patchset[1] to
> map non-PCDIMM devices to guest address space.
This sentence doesn't belong in git history. It is better to put
information like this...
> There is still upstream discussion on using
> among PCI bar vs memory device, will update
> as per concensus.
s/concensus/consensus/
>
> [1] https://marc.info/?l=qemu-devel=152450249319168=2
>
> Signed-off-by: Pankaj Gupta
> ---
...here, where it is part of the email, but not picked up by 'git am'.
> +++ b/qapi/misc.json
> @@ -2871,6 +2871,29 @@
>}
> }
>
> +##
> +# @VirtioPMemDeviceInfo:
> +#
> +# VirtioPMem state information
> +#
> +# @id: device's ID
> +#
> +# @start: physical address, where device is mapped
> +#
> +# @size: size of memory that the device provides
> +#
> +# @memdev: memory backend linked with device
> +#
> +# Since: 2.13
> +##
> +{ 'struct': 'VirtioPMemDeviceInfo',
> +'data': { '*id': 'str',
> + 'start': 'size',
> + 'size': 'size',
TAB damage.
> + 'memdev': 'str'
> + }
> +}
> +
> ##
> # @MemoryDeviceInfo:
> #
> @@ -2880,7 +2903,8 @@
> ##
> { 'union': 'MemoryDeviceInfo',
>'data': { 'dimm': 'PCDIMMDeviceInfo',
> -'nvdimm': 'PCDIMMDeviceInfo'
> +'nvdimm': 'PCDIMMDeviceInfo',
> + 'virtio-pmem': 'VirtioPMemDeviceInfo'
>}
> }
>
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [RFC v2] qemu: Add virtio pmem device
On 04/25/2018 06:58 AM, Pankaj Gupta wrote: > > Hi, > > Compile failures are because Qemu 'Memory-Device changes' are not yet > in qemu master. As mentioned in Qemu patch message patch is > dependent on 'Memeory-device' patches by 'David Hildenbrand'. On 04/25/2018 06:24 AM, Pankaj Gupta wrote: > This PV device code is dependent and tested > with 'David Hildenbrand's ' patchset[1] to > map non-PCDIMM devices to guest address space. > There is still upstream discussion on using > among PCI bar vs memory device, will update > as per concensus. > > [1] https://marc.info/?l=qemu-devel=152450249319168=2 Then let's spell that in a way that patchew understands (since patchew does not know how to turn marc.info references into Message-IDs): Based-on: <20180423165126.15441-1-da...@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [RFC v2] qemu: Add virtio pmem device
On 04/25/2018 06:58 AM, Pankaj Gupta wrote: > > Hi, > > Compile failures are because Qemu 'Memory-Device changes' are not yet > in qemu master. As mentioned in Qemu patch message patch is > dependent on 'Memeory-device' patches by 'David Hildenbrand'. On 04/25/2018 06:24 AM, Pankaj Gupta wrote: > This PV device code is dependent and tested > with 'David Hildenbrand's ' patchset[1] to > map non-PCDIMM devices to guest address space. > There is still upstream discussion on using > among PCI bar vs memory device, will update > as per concensus. > > [1] https://marc.info/?l=qemu-devel=152450249319168=2 Then let's spell that in a way that patchew understands (since patchew does not know how to turn marc.info references into Message-IDs): Based-on: <20180423165126.15441-1-da...@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] (v2. forward to qemu )-Panic with ext4, nbd, qemu-img, block
On 01/21/2018 08:06 PM, Hongzhi, Song wrote: > Hello, > > I create a virtual disk-image using qemu-img. > > And then I use /dev/nbd to map the image. > > I mount the /dev/nbd to a local dir with ext4-format > > Finally, I have some trouble about ext4-filesystem and block device, > with using demand of rsync or dd to write the image. > > Reproduce : > > qemu-img create test.img 2G > > mkfs.ext4 -F test.img > > qemu-nbd -f raw -c /dev/nbd0 test.img > > mount -r ext4 /dev/nbd0 LOCAL_DIR/ > > rsync -av META_DATA_DIR/ LOCAL_DIR/ > > Qemu Version: > > QEMU emulator version 2.10.0 There have been some bug fixes in the NBD code in qemu 2.11; does using a newer version make a difference in your results? > Detail: > > > 329.11 EXT4-fs (nbd0): mounted filesystem with ordered data mode. Opts: > (null) > 329.12 block nbd0: Connection timed out > 329.13 block nbd0: shutting down sockets This sounds like a log of the kernel side; but it is rather sparse on details on why the kernel lost the connection to the socket provided by qemu-nbd -c. Is there any chance we can get a corresponding trace from qemu-nbd when reproducing the lost connection? > 329.14 blk_update_request: I/O error, dev nbd0, sector 304384 > 329.15 blk_update_request: I/O error, dev nbd0, sector 304640 > 329.16 blk_update_request: I/O error, dev nbd0, sector 304896 > 329.17 blk_update_request: I/O error, dev nbd0, sector 305152 > 329.18 blk_update_request: I/O error, dev nbd0, sector 305408 > 329.19 blk_update_request: I/O error, dev nbd0, sector 305664 > 329.20 blk_update_request: I/O error, dev nbd0, sector 305920 > 329.21 blk_update_request: I/O error, dev nbd0, sector 306176 > 329.22 blk_update_request: I/O error, dev nbd0, sector 306432 > 329.23 blk_update_request: I/O error, dev nbd0, sector 306688 > 329.24 EXT4-fs warning (device nbd0): ext4_end_bio:322: I/O error -5 > writing to inode 160 (offset 8388608 size 8388608 starting block 38400) Everything else in the trace looks like fallout from the initial lost connection - once the kernel can't communicate to the NBD server, it has to fail all pending and subsequent I/O requests to /dev/nbd0. But until we can figure out why the connection is dropped, seeing this part of the trace doesn't add any information about the root cause. But oddly enough, once things go south in the kernel nbd module, it leads to a full-on kernel bug: > GRNDSDP1.86B.0036.R05.1407140519 07/14/2014 > 329.51 Workqueue: writeback wb_workfn (flush-43:0) > 329.52 task: 977bec759e00 task.stack: a2930524c000 > 329.53 RIP: 0010:submit_bh_wbc+0x155/0x160 > 329.54 RSP: 0018:a2930524f7e0 EFLAGS: 00010246 > 329.55 RAX: 00620005 RBX: 977f05cddc18 RCX: > 329.56 RDX: 977f05cddc18 RSI: 00020800 RDI: 0001 > 329.57 RBP: a2930524f808 R08: ff00 R09: 00ff > 329.58 R10: a2930524f920 R11: 058c R12: a598 > 329.59 R13: ba15c500 R14: 977fe1bab400 R15: 977fea643000 > 329.60 FS: () GS:977befa0() > knlGS: > 329.61 CS: 0010 DS: ES: CR0: 80050033 > 329.62 CR2: 7f7d7010 CR3: 00035ce0e000 CR4: 001406e0 > 329.63 Call Trace: > 329.64 __sync_dirty_buffer+0x41/0xa0 > 329.65 ext4_commit_super+0x1d6/0x2a0 > 329.66 __ext4_error_inode+0xb2/0x170 > 329.99 JBD2: Error -5 detected when updating journal superblock for nbd0-8. > 329.100 Aborting journal on device nbd0-8. > 329.101 [ cut here ] > 329.102 kernel BUG at /kernel-source//fs/buffer.c:3091! Well, that should certainly be reported to the kernel folks; nothing qemu can do about it (a userspace socket serving NBD data should not be able to cause the kernel NBD client to result in a subsequent kernel crash, regardless of how bad data loss is when the socket disappears out from under the kernel). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] (v2. forward to qemu )-Panic with ext4, nbd, qemu-img, block
On 01/21/2018 08:06 PM, Hongzhi, Song wrote: > Hello, > > I create a virtual disk-image using qemu-img. > > And then I use /dev/nbd to map the image. > > I mount the /dev/nbd to a local dir with ext4-format > > Finally, I have some trouble about ext4-filesystem and block device, > with using demand of rsync or dd to write the image. > > Reproduce : > > qemu-img create test.img 2G > > mkfs.ext4 -F test.img > > qemu-nbd -f raw -c /dev/nbd0 test.img > > mount -r ext4 /dev/nbd0 LOCAL_DIR/ > > rsync -av META_DATA_DIR/ LOCAL_DIR/ > > Qemu Version: > > QEMU emulator version 2.10.0 There have been some bug fixes in the NBD code in qemu 2.11; does using a newer version make a difference in your results? > Detail: > > > 329.11 EXT4-fs (nbd0): mounted filesystem with ordered data mode. Opts: > (null) > 329.12 block nbd0: Connection timed out > 329.13 block nbd0: shutting down sockets This sounds like a log of the kernel side; but it is rather sparse on details on why the kernel lost the connection to the socket provided by qemu-nbd -c. Is there any chance we can get a corresponding trace from qemu-nbd when reproducing the lost connection? > 329.14 blk_update_request: I/O error, dev nbd0, sector 304384 > 329.15 blk_update_request: I/O error, dev nbd0, sector 304640 > 329.16 blk_update_request: I/O error, dev nbd0, sector 304896 > 329.17 blk_update_request: I/O error, dev nbd0, sector 305152 > 329.18 blk_update_request: I/O error, dev nbd0, sector 305408 > 329.19 blk_update_request: I/O error, dev nbd0, sector 305664 > 329.20 blk_update_request: I/O error, dev nbd0, sector 305920 > 329.21 blk_update_request: I/O error, dev nbd0, sector 306176 > 329.22 blk_update_request: I/O error, dev nbd0, sector 306432 > 329.23 blk_update_request: I/O error, dev nbd0, sector 306688 > 329.24 EXT4-fs warning (device nbd0): ext4_end_bio:322: I/O error -5 > writing to inode 160 (offset 8388608 size 8388608 starting block 38400) Everything else in the trace looks like fallout from the initial lost connection - once the kernel can't communicate to the NBD server, it has to fail all pending and subsequent I/O requests to /dev/nbd0. But until we can figure out why the connection is dropped, seeing this part of the trace doesn't add any information about the root cause. But oddly enough, once things go south in the kernel nbd module, it leads to a full-on kernel bug: > GRNDSDP1.86B.0036.R05.1407140519 07/14/2014 > 329.51 Workqueue: writeback wb_workfn (flush-43:0) > 329.52 task: 977bec759e00 task.stack: a2930524c000 > 329.53 RIP: 0010:submit_bh_wbc+0x155/0x160 > 329.54 RSP: 0018:a2930524f7e0 EFLAGS: 00010246 > 329.55 RAX: 00620005 RBX: 977f05cddc18 RCX: > 329.56 RDX: 977f05cddc18 RSI: 00020800 RDI: 0001 > 329.57 RBP: a2930524f808 R08: ff00 R09: 00ff > 329.58 R10: a2930524f920 R11: 058c R12: a598 > 329.59 R13: ba15c500 R14: 977fe1bab400 R15: 977fea643000 > 329.60 FS: () GS:977befa0() > knlGS: > 329.61 CS: 0010 DS: ES: CR0: 80050033 > 329.62 CR2: 7f7d7010 CR3: 00035ce0e000 CR4: 001406e0 > 329.63 Call Trace: > 329.64 __sync_dirty_buffer+0x41/0xa0 > 329.65 ext4_commit_super+0x1d6/0x2a0 > 329.66 __ext4_error_inode+0xb2/0x170 > 329.99 JBD2: Error -5 detected when updating journal superblock for nbd0-8. > 329.100 Aborting journal on device nbd0-8. > 329.101 [ cut here ] > 329.102 kernel BUG at /kernel-source//fs/buffer.c:3091! Well, that should certainly be reported to the kernel folks; nothing qemu can do about it (a userspace socket serving NBD data should not be able to cause the kernel NBD client to result in a subsequent kernel crash, regardless of how bad data loss is when the socket disappears out from under the kernel). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [PATCH 1/2] vfs: implement fchmodat2() syscall
On 04/11/2017 01:07 PM, Eric Blake wrote: > > But even if using open(O_PATH)/fchmod() works, it is not immediately > obvious whether it can catch all the same cases that chmodat(O_NOFOLLOW) Typo; I obviously meant fchmodat(AT_SYMLINK_NOFOLLOW) > would cover, as there are cases where you have permissions to change > mode bits but not open() the file for reading or writing. And even if > it gets rid of a TOCTTOU race, it still is a 2-syscall hit rather than > an atomic single syscall. > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [PATCH 1/2] vfs: implement fchmodat2() syscall
On 04/11/2017 01:07 PM, Eric Blake wrote: > > But even if using open(O_PATH)/fchmod() works, it is not immediately > obvious whether it can catch all the same cases that chmodat(O_NOFOLLOW) Typo; I obviously meant fchmodat(AT_SYMLINK_NOFOLLOW) > would cover, as there are cases where you have permissions to change > mode bits but not open() the file for reading or writing. And even if > it gets rid of a TOCTTOU race, it still is a 2-syscall hit rather than > an atomic single syscall. > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [PATCH 1/2] vfs: implement fchmodat2() syscall
On 04/11/2017 12:55 PM, Eric Blake wrote: > On 04/11/2017 12:52 PM, Colin Walters wrote: >> >> >> On Tue, Feb 28, 2017, at 02:23 PM, Eric Blake wrote: >> >>> Might also be worth mentioning that this patch is required in order to >>> solve CVE-2016-9602, per discussion at >>> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg06089.html >> >> I only briefly looked at this, but can't `open(..., O_PATH)` be used to solve >> this today? > > O_PATH was the fallback that qemu used Hmm - actually, qemu used O_PATH for the directory portion of *at traversals: git.qemu-project.org/?p=qemu.git;a=commitdiff;h=918112c but did not use O_PATH for its chmod() fallback: git.qemu-project.org/?p=qemu.git;a=commitdiff;h=e3187a4 A good idea on the surface. But reading the man page of openat(), the section on O_PATH says: The file itself is not opened, and other file operations (e.g., read(2), write(2), fchmod(2), fchown(2), fgetxattr(2), mmap(2)) fail with the error EBADF. > - but that's non-POSIX, which > means we have to have a different solution for POSIX systems than for > Linux systems, while waiting for Linux to catch up to POSIX. But even if using open(O_PATH)/fchmod() works, it is not immediately obvious whether it can catch all the same cases that chmodat(O_NOFOLLOW) would cover, as there are cases where you have permissions to change mode bits but not open() the file for reading or writing. And even if it gets rid of a TOCTTOU race, it still is a 2-syscall hit rather than an atomic single syscall. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [PATCH 1/2] vfs: implement fchmodat2() syscall
On 04/11/2017 12:55 PM, Eric Blake wrote: > On 04/11/2017 12:52 PM, Colin Walters wrote: >> >> >> On Tue, Feb 28, 2017, at 02:23 PM, Eric Blake wrote: >> >>> Might also be worth mentioning that this patch is required in order to >>> solve CVE-2016-9602, per discussion at >>> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg06089.html >> >> I only briefly looked at this, but can't `open(..., O_PATH)` be used to solve >> this today? > > O_PATH was the fallback that qemu used Hmm - actually, qemu used O_PATH for the directory portion of *at traversals: git.qemu-project.org/?p=qemu.git;a=commitdiff;h=918112c but did not use O_PATH for its chmod() fallback: git.qemu-project.org/?p=qemu.git;a=commitdiff;h=e3187a4 A good idea on the surface. But reading the man page of openat(), the section on O_PATH says: The file itself is not opened, and other file operations (e.g., read(2), write(2), fchmod(2), fchown(2), fgetxattr(2), mmap(2)) fail with the error EBADF. > - but that's non-POSIX, which > means we have to have a different solution for POSIX systems than for > Linux systems, while waiting for Linux to catch up to POSIX. But even if using open(O_PATH)/fchmod() works, it is not immediately obvious whether it can catch all the same cases that chmodat(O_NOFOLLOW) would cover, as there are cases where you have permissions to change mode bits but not open() the file for reading or writing. And even if it gets rid of a TOCTTOU race, it still is a 2-syscall hit rather than an atomic single syscall. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [PATCH 1/2] vfs: implement fchmodat2() syscall
On 04/11/2017 12:52 PM, Colin Walters wrote: > > > On Tue, Feb 28, 2017, at 02:23 PM, Eric Blake wrote: > >> Might also be worth mentioning that this patch is required in order to >> solve CVE-2016-9602, per discussion at >> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg06089.html > > I only briefly looked at this, but can't `open(..., O_PATH)` be used to solve > this today? O_PATH was the fallback that qemu used - but that's non-POSIX, which means we have to have a different solution for POSIX systems than for Linux systems, while waiting for Linux to catch up to POSIX. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [PATCH 1/2] vfs: implement fchmodat2() syscall
On 04/11/2017 12:52 PM, Colin Walters wrote: > > > On Tue, Feb 28, 2017, at 02:23 PM, Eric Blake wrote: > >> Might also be worth mentioning that this patch is required in order to >> solve CVE-2016-9602, per discussion at >> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg06089.html > > I only briefly looked at this, but can't `open(..., O_PATH)` be used to solve > this today? O_PATH was the fallback that qemu used - but that's non-POSIX, which means we have to have a different solution for POSIX systems than for Linux systems, while waiting for Linux to catch up to POSIX. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [PATCH 1/2] vfs: implement fchmodat2() syscall
On 02/28/2017 12:41 PM, Greg Kurz wrote: >>> +++ b/include/linux/syscalls.h >>> @@ -775,6 +775,8 @@ asmlinkage long sys_futimesat(int dfd, const char >>> __user *filename, >>> asmlinkage long sys_faccessat(int dfd, const char __user *filename, int >>> mode); >>> asmlinkage long sys_fchmodat(int dfd, const char __user * filename, >>> umode_t mode); >>> +asmlinkage long sys_fchmodat2(int dfd, const char __user *filename, >>> + umode_t mode, int flag); >>> asmlinkage long sys_fchownat(int dfd, const char __user *filename, uid_t >>> user, >>> gid_t group, int flag); >> >> Is the indentation off here? >> > > This is linux style indent with tabs+spaces. FWIW it is displayed correctly > in vi and emacs (I've simply copied the sys_fchmodat() declaration). Sorry for the noise; I see that it is correct now, since fchmodat2 is one bye longer than fchmodat or fchownat. Sometimes, I wish I could convince my mailer to display leading tabs as exactly 8 spaces rather than to the next 8-space tab stop, so that prefixed characters that aren't tab-stop aligned don't mess with the actual visual output. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [PATCH 1/2] vfs: implement fchmodat2() syscall
On 02/28/2017 12:41 PM, Greg Kurz wrote: >>> +++ b/include/linux/syscalls.h >>> @@ -775,6 +775,8 @@ asmlinkage long sys_futimesat(int dfd, const char >>> __user *filename, >>> asmlinkage long sys_faccessat(int dfd, const char __user *filename, int >>> mode); >>> asmlinkage long sys_fchmodat(int dfd, const char __user * filename, >>> umode_t mode); >>> +asmlinkage long sys_fchmodat2(int dfd, const char __user *filename, >>> + umode_t mode, int flag); >>> asmlinkage long sys_fchownat(int dfd, const char __user *filename, uid_t >>> user, >>> gid_t group, int flag); >> >> Is the indentation off here? >> > > This is linux style indent with tabs+spaces. FWIW it is displayed correctly > in vi and emacs (I've simply copied the sys_fchmodat() declaration). Sorry for the noise; I see that it is correct now, since fchmodat2 is one bye longer than fchmodat or fchownat. Sometimes, I wish I could convince my mailer to display leading tabs as exactly 8 spaces rather than to the next 8-space tab stop, so that prefixed characters that aren't tab-stop aligned don't mess with the actual visual output. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [PATCH 1/2] vfs: implement fchmodat2() syscall
On 02/28/2017 11:03 AM, Greg Kurz wrote: > According to the POSIX.1-2008 manual page [1], the fchmodat() function has > a flag argument which may be passed the following value: > > AT_SYMLINK_NOFOLLOW > If path names a symbolic link, then the mode of the symbolic link is > changed. > > and the following error may be returned: > > [EOPNOTSUPP] > The AT_SYMLINK_NOFOLLOW bit is set in the flag argument, path names a > symbolic link, and the system does not support changing the mode of a > symbolic link. > > The linux kernel doesn't support changing the mode of a symbolic link, but > the current implementation doesn't even have a flag argument. It is then > up to userspace to deal with that. Unfortunately, it is impossible to > implement the POSIX behavior in a race-free manner. > > This patch introduces a new fchmodat2() syscall with a flag argument to > address the issue. > > [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/chmod.html > > Signed-off-by: Greg Kurz <gr...@kaod.org> > --- Might also be worth mentioning that this patch is required in order to solve CVE-2016-9602, per discussion at https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg06089.html > +++ b/include/linux/syscalls.h > @@ -775,6 +775,8 @@ asmlinkage long sys_futimesat(int dfd, const char __user > *filename, > asmlinkage long sys_faccessat(int dfd, const char __user *filename, int > mode); > asmlinkage long sys_fchmodat(int dfd, const char __user * filename, >umode_t mode); > +asmlinkage long sys_fchmodat2(int dfd, const char __user *filename, > + umode_t mode, int flag); > asmlinkage long sys_fchownat(int dfd, const char __user *filename, uid_t > user, > gid_t group, int flag); Is the indentation off here? Reviewed-by: Eric Blake <ebl...@redhat.com> -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [PATCH 1/2] vfs: implement fchmodat2() syscall
On 02/28/2017 11:03 AM, Greg Kurz wrote: > According to the POSIX.1-2008 manual page [1], the fchmodat() function has > a flag argument which may be passed the following value: > > AT_SYMLINK_NOFOLLOW > If path names a symbolic link, then the mode of the symbolic link is > changed. > > and the following error may be returned: > > [EOPNOTSUPP] > The AT_SYMLINK_NOFOLLOW bit is set in the flag argument, path names a > symbolic link, and the system does not support changing the mode of a > symbolic link. > > The linux kernel doesn't support changing the mode of a symbolic link, but > the current implementation doesn't even have a flag argument. It is then > up to userspace to deal with that. Unfortunately, it is impossible to > implement the POSIX behavior in a race-free manner. > > This patch introduces a new fchmodat2() syscall with a flag argument to > address the issue. > > [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/chmod.html > > Signed-off-by: Greg Kurz > --- Might also be worth mentioning that this patch is required in order to solve CVE-2016-9602, per discussion at https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg06089.html > +++ b/include/linux/syscalls.h > @@ -775,6 +775,8 @@ asmlinkage long sys_futimesat(int dfd, const char __user > *filename, > asmlinkage long sys_faccessat(int dfd, const char __user *filename, int > mode); > asmlinkage long sys_fchmodat(int dfd, const char __user * filename, >umode_t mode); > +asmlinkage long sys_fchmodat2(int dfd, const char __user *filename, > + umode_t mode, int flag); > asmlinkage long sys_fchownat(int dfd, const char __user *filename, uid_t > user, > gid_t group, int flag); Is the indentation off here? Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] drm: update MAINTAINERS for qemu drivers (bochs, cirrus, qxl, virtio-gpu)
On 11/22/2016 01:41 PM, Paolo Bonzini wrote: > > > On 22/11/2016 19:54, Eric Blake wrote: >>>>>> DRM DRIVER FOR BOCHS VIRTUAL GPU >>>>>> M: Gerd Hoffmann <kra...@redhat.com> >>>>>> -S: Odd Fixes >>>>>> +L: qemu-de...@nongnu.org >>>> >>>> qemu-devel list already has very high traffic - not sure whether it >>>> makes much sense to route even more additional patches here. Maybe >>>> rather create a separate mailing list like qemu-graph...@nongnu.org ? >> In practice, ALL patches should already be going to qemu-devel, even if >> they are ALSO going to some other list. For example, qemu-block and >> qemu-trivial are definitely cases where we have separate lists, but >> where posters are reminded to include qemu-devel in cc if they want a >> patch applied. > > The difference is that these would be kernel patches. Ah, indeed. I missed the distinction of 'all _qemu_ patches' are already going to the qemu list, but this is about non-qemu patches. I guess I was thrown off because I first read this message as cross-posted onto the qemu lists, rather than its primary audience of the kernel list. But it goes to show that when more than one non-overlapping list is cc'd on an individual patch, it gets harder to tell which list the patch is meant for, vs. which other lists are just getting it out of courtesy. So you've just managed to convince me that including qemu-devel on every driver patch, when qemu.git will not be modified, may indeed be overkill; when compared to the option of just creating a new dedicated list for the subset of kernel patches related to qemu drivers. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] drm: update MAINTAINERS for qemu drivers (bochs, cirrus, qxl, virtio-gpu)
On 11/22/2016 01:41 PM, Paolo Bonzini wrote: > > > On 22/11/2016 19:54, Eric Blake wrote: >>>>>> DRM DRIVER FOR BOCHS VIRTUAL GPU >>>>>> M: Gerd Hoffmann >>>>>> -S: Odd Fixes >>>>>> +L: qemu-de...@nongnu.org >>>> >>>> qemu-devel list already has very high traffic - not sure whether it >>>> makes much sense to route even more additional patches here. Maybe >>>> rather create a separate mailing list like qemu-graph...@nongnu.org ? >> In practice, ALL patches should already be going to qemu-devel, even if >> they are ALSO going to some other list. For example, qemu-block and >> qemu-trivial are definitely cases where we have separate lists, but >> where posters are reminded to include qemu-devel in cc if they want a >> patch applied. > > The difference is that these would be kernel patches. Ah, indeed. I missed the distinction of 'all _qemu_ patches' are already going to the qemu list, but this is about non-qemu patches. I guess I was thrown off because I first read this message as cross-posted onto the qemu lists, rather than its primary audience of the kernel list. But it goes to show that when more than one non-overlapping list is cc'd on an individual patch, it gets harder to tell which list the patch is meant for, vs. which other lists are just getting it out of courtesy. So you've just managed to convince me that including qemu-devel on every driver patch, when qemu.git will not be modified, may indeed be overkill; when compared to the option of just creating a new dedicated list for the subset of kernel patches related to qemu drivers. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] drm: update MAINTAINERS for qemu drivers (bochs, cirrus, qxl, virtio-gpu)
On 11/22/2016 04:50 AM, Thomas Huth wrote: > On 22.11.2016 10:58, Gerd Hoffmann wrote: >> Changes: >> * add myself as maintainer, so patches land in my inbox. >> * add qemu-devel mailing list. >> * add drm-qemu git repo. >> * flip bochs and qxl status to "Maintained". >> >> Signed-off-by: Gerd Hoffmann <kra...@redhat.com> >> --- >> MAINTAINERS | 14 -- >> 1 file changed, 12 insertions(+), 2 deletions(-) >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index ad9b965..84dc747 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -4027,11 +4027,16 @@ F: drivers/gpu/drm/ast/ >> >> DRM DRIVER FOR BOCHS VIRTUAL GPU >> M: Gerd Hoffmann <kra...@redhat.com> >> -S: Odd Fixes >> +L: qemu-de...@nongnu.org > > qemu-devel list already has very high traffic - not sure whether it > makes much sense to route even more additional patches here. Maybe > rather create a separate mailing list like qemu-graph...@nongnu.org ? In practice, ALL patches should already be going to qemu-devel, even if they are ALSO going to some other list. For example, qemu-block and qemu-trivial are definitely cases where we have separate lists, but where posters are reminded to include qemu-devel in cc if they want a patch applied. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] drm: update MAINTAINERS for qemu drivers (bochs, cirrus, qxl, virtio-gpu)
On 11/22/2016 04:50 AM, Thomas Huth wrote: > On 22.11.2016 10:58, Gerd Hoffmann wrote: >> Changes: >> * add myself as maintainer, so patches land in my inbox. >> * add qemu-devel mailing list. >> * add drm-qemu git repo. >> * flip bochs and qxl status to "Maintained". >> >> Signed-off-by: Gerd Hoffmann >> --- >> MAINTAINERS | 14 -- >> 1 file changed, 12 insertions(+), 2 deletions(-) >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index ad9b965..84dc747 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -4027,11 +4027,16 @@ F: drivers/gpu/drm/ast/ >> >> DRM DRIVER FOR BOCHS VIRTUAL GPU >> M: Gerd Hoffmann >> -S: Odd Fixes >> +L: qemu-de...@nongnu.org > > qemu-devel list already has very high traffic - not sure whether it > makes much sense to route even more additional patches here. Maybe > rather create a separate mailing list like qemu-graph...@nongnu.org ? In practice, ALL patches should already be going to qemu-devel, even if they are ALSO going to some other list. For example, qemu-block and qemu-trivial are definitely cases where we have separate lists, but where posters are reminded to include qemu-devel in cc if they want a patch applied. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Nbd] [RESEND][PATCH 0/5] nbd improvements
On 09/15/2016 11:27 AM, Wouter Verhelst wrote: > On Thu, Sep 15, 2016 at 05:08:21PM +0100, Alex Bligh wrote: >> Wouter, >> >>> The server can always refuse to allow multiple connections. >> >> Sure, but it would be neater to warn the client of that at negotiation >> stage (it would only be one flag, e.g. 'multiple connections >> unsafe'). > > I suppose that's not a bad idea. Seems like it may need to be a per-export flag, rather than a global flag (as a given server may be able to serve multiple types of files, where the safety depends on the type of file being served). -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Nbd] [RESEND][PATCH 0/5] nbd improvements
On 09/15/2016 11:27 AM, Wouter Verhelst wrote: > On Thu, Sep 15, 2016 at 05:08:21PM +0100, Alex Bligh wrote: >> Wouter, >> >>> The server can always refuse to allow multiple connections. >> >> Sure, but it would be neater to warn the client of that at negotiation >> stage (it would only be one flag, e.g. 'multiple connections >> unsafe'). > > I suppose that's not a bad idea. Seems like it may need to be a per-export flag, rather than a global flag (as a given server may be able to serve multiple types of files, where the safety depends on the type of file being served). -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Nbd] [RESEND][PATCH 0/5] nbd improvements
On 09/15/2016 06:09 AM, Alex Bligh wrote: > > I also wonder whether any servers that can do caching per > connection will always share a consistent cache between > connections. The one I'm worried about in particular here > is qemu-nbd - Eric Blake CC'd. > I doubt that qemu-nbd would ever want to support the situation with more than one client connection writing to the same image at the same time; the implications of sorting out data consistency between multiple writers is rather complex and not worth coding into qemu. So I think qemu would probably prefer to just prohibit the multiple writer situation. And while multiple readers with no writer should be fine, I'm not even sure if multiple readers plus one writer can always be made to appear sane (if there is no coordination between the different connections, on an image where the writer changes AA to BA then flushes then changes to BB, it is still feasible that a reader could see AB (pre-flush state of the first sector, post-flush changes to the second sector, even though the writer never flushed that particular content to disk). But Paolo Bonzini (cc'd) may have more insight on qemu's NBD server and what it supports (or forbids) in the way of multiple clients to a single server. > A more general point is that with multiple queues requests > may be processed in a different order even by those servers that > currently process the requests in strict order, or in something > similar to strict order. The server is permitted by the spec > (save as mandated by NBD_CMD_FLUSH and NBD_CMD_FLAG_FUA) to > process commands out of order anyway, but I suspect this has > to date been little tested. qemu-nbd is definitely capable of serving reads and writes out-of-order to a single connection client; but that's different than the case with multiple connections. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Nbd] [RESEND][PATCH 0/5] nbd improvements
On 09/15/2016 06:09 AM, Alex Bligh wrote: > > I also wonder whether any servers that can do caching per > connection will always share a consistent cache between > connections. The one I'm worried about in particular here > is qemu-nbd - Eric Blake CC'd. > I doubt that qemu-nbd would ever want to support the situation with more than one client connection writing to the same image at the same time; the implications of sorting out data consistency between multiple writers is rather complex and not worth coding into qemu. So I think qemu would probably prefer to just prohibit the multiple writer situation. And while multiple readers with no writer should be fine, I'm not even sure if multiple readers plus one writer can always be made to appear sane (if there is no coordination between the different connections, on an image where the writer changes AA to BA then flushes then changes to BB, it is still feasible that a reader could see AB (pre-flush state of the first sector, post-flush changes to the second sector, even though the writer never flushed that particular content to disk). But Paolo Bonzini (cc'd) may have more insight on qemu's NBD server and what it supports (or forbids) in the way of multiple clients to a single server. > A more general point is that with multiple queues requests > may be processed in a different order even by those servers that > currently process the requests in strict order, or in something > similar to strict order. The server is permitted by the spec > (save as mandated by NBD_CMD_FLUSH and NBD_CMD_FLAG_FUA) to > process commands out of order anyway, but I suspect this has > to date been little tested. qemu-nbd is definitely capable of serving reads and writes out-of-order to a single connection client; but that's different than the case with multiple connections. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Nbd] [PATCH 3/3]nbd: make nbd device wait for its users
On 06/24/2016 04:09 AM, Pranay Kr. Srivastava wrote: > When a timeout occurs or a recv fails, then > instead of abruplty killing nbd block device s/abruplty/abruptly/ > wait for it's users to finish. s/it's/its/ > > This is more required when filesystem(s) like > ext2 or ext3 don't expect their buffer heads to > disappear while the filesystem is mounted. > > Each open of a nbd device is refcounted, while > the userland program [nbd-client] doing the > NBD_DO_IT ioctl would now wait for any other users > of this device before invalidating the nbd device. > > A timedout or a disconnected device, if in use, can't > be used until it has been resetted. The resetting happens s/resetted/reset/ > when all tasks having this bdev open closes this bdev. > > Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> > --- > drivers/block/nbd.c | 124 > ---- > 1 file changed, 96 insertions(+), 28 deletions(-) > -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Nbd] [PATCH 3/3]nbd: make nbd device wait for its users
On 06/24/2016 04:09 AM, Pranay Kr. Srivastava wrote: > When a timeout occurs or a recv fails, then > instead of abruplty killing nbd block device s/abruplty/abruptly/ > wait for it's users to finish. s/it's/its/ > > This is more required when filesystem(s) like > ext2 or ext3 don't expect their buffer heads to > disappear while the filesystem is mounted. > > Each open of a nbd device is refcounted, while > the userland program [nbd-client] doing the > NBD_DO_IT ioctl would now wait for any other users > of this device before invalidating the nbd device. > > A timedout or a disconnected device, if in use, can't > be used until it has been resetted. The resetting happens s/resetted/reset/ > when all tasks having this bdev open closes this bdev. > > Signed-off-by: Pranay Kr. Srivastava > --- > drivers/block/nbd.c | 124 > > 1 file changed, 96 insertions(+), 28 deletions(-) > -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Nbd] [PATCH 1/2] nbd: make nbd device wait for its users
On 06/24/2016 03:29 AM, Markus Pargmann wrote: > From: "Pranay Kr. Srivastava" <pran...@gmail.com> > > When a timeout occurs or a recv fails, then instead of abruplty killing s/abruplty/abruptly/ > nbd block device wait for it's users to finish. s/it's/its/ (remember, "it's" is only usable where "it is" would also be appropriate) > > This is more required when filesystem(s) like ext2 or ext3 don't expect > their buffer heads to disappear while the filesystem is mounted. > > Each open is counted. The blockdevice is kept open until the last user > closes the block device. This offers the possibility as well to open a > new socket to be used while the filesystems are mounted. > > Signed-off-by: Pranay Kr. Srivastava <pran...@gmail.com> > > [mpa: Keep the blockdevice open until all users left] > Signed-off-by: Markus Pargmann <m...@pengutronix.de> -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Nbd] [PATCH 1/2] nbd: make nbd device wait for its users
On 06/24/2016 03:29 AM, Markus Pargmann wrote: > From: "Pranay Kr. Srivastava" > > When a timeout occurs or a recv fails, then instead of abruplty killing s/abruplty/abruptly/ > nbd block device wait for it's users to finish. s/it's/its/ (remember, "it's" is only usable where "it is" would also be appropriate) > > This is more required when filesystem(s) like ext2 or ext3 don't expect > their buffer heads to disappear while the filesystem is mounted. > > Each open is counted. The blockdevice is kept open until the last user > closes the block device. This offers the possibility as well to open a > new socket to be used while the filesystems are mounted. > > Signed-off-by: Pranay Kr. Srivastava > > [mpa: Keep the blockdevice open until all users left] > Signed-off-by: Markus Pargmann -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Nbd] Fwd: [PATCH v4 02/18] nbd: fix checkpatch trailing space warning.
On 05/11/2016 03:38 AM, Pranay Srivastava wrote: > > The series contained some checkpatch changes so I had included you as well. > >> know why you are sending them to me), but I know I do not accept patches >> without any changelog text at all in them, as that's just lazy. > > That should be per patch or can appear in a cover letter for the patches? Per patch. However, if it were me, I would not have split into quite so many patches. The mantra is one patch per one fix, but I think it is reasonable to state that "silence all checkpatch warnings" counts as one fix, rather than 16 separate fixes. If you DO consolidate the checkpatch changes into a single patch, then the commit message body should call out a bulleted list of all the changes you are making, as well as a justification why it is worth churning the entire file rather than just making smaller checkpatch fixes in just the areas that your other patches touch. > > Actually I've made more patches in this series after I had sent the > earlier ones, > but the earlier ones are not changed at all. It's only the addition of > newer patches > to the series. The cover letter is a great place to point out how v4 differs from v3, but also to point out which patches are unchanged from v3, to save reviewer's time. So if all you did was add new patches, a cover-letter mention of which patches remain unchanged might be helpful. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Nbd] Fwd: [PATCH v4 02/18] nbd: fix checkpatch trailing space warning.
On 05/11/2016 03:38 AM, Pranay Srivastava wrote: > > The series contained some checkpatch changes so I had included you as well. > >> know why you are sending them to me), but I know I do not accept patches >> without any changelog text at all in them, as that's just lazy. > > That should be per patch or can appear in a cover letter for the patches? Per patch. However, if it were me, I would not have split into quite so many patches. The mantra is one patch per one fix, but I think it is reasonable to state that "silence all checkpatch warnings" counts as one fix, rather than 16 separate fixes. If you DO consolidate the checkpatch changes into a single patch, then the commit message body should call out a bulleted list of all the changes you are making, as well as a justification why it is worth churning the entire file rather than just making smaller checkpatch fixes in just the areas that your other patches touch. > > Actually I've made more patches in this series after I had sent the > earlier ones, > but the earlier ones are not changed at all. It's only the addition of > newer patches > to the series. The cover letter is a great place to point out how v4 differs from v3, but also to point out which patches are unchanged from v3, to save reviewer's time. So if all you did was add new patches, a cover-letter mention of which patches remain unchanged might be helpful. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v5 1/4] firmware: introduce sysfs driver for QEMU's fw_cfg device
On 11/24/2015 09:55 AM, Gabriel L. Somlo wrote: > On Tue, Nov 24, 2015 at 04:14:50AM +0800, kbuild test robot wrote: >> >>drivers/firmware/qemu_fw_cfg.c: In function 'fw_cfg_cmdline_set': >>>> drivers/firmware/qemu_fw_cfg.c:510:7: warning: format '%lli' expects >>>> argument of type 'long long int *', but argument 3 has type 'phys_addr_t >>>> *' [-Wformat=] >> _off, _off, ); >> ^ > > Oh, I think I know why this happened: > > > So, I could use u64 instead of phys_addr_t and resource_size_t, and > keep "%lli" (or "%Li"), but then I'd have to check if the parsed value %Li is not POSIX. Don't use it (stick with %lli). > would overflow a 32-bit address value on arches where phys_addr_t is > u32, which would make things a bit more messy and awkward. > > I'm planning on #ifdef-ing the format string instead: > > #ifdef CONFIG_PHYS_ADDR_T_64BIT > #define PH_ADDR_SCAN_FMT "@%Li%n:%Li:%Li%n" > #else > #define PH_ADDR_SCAN_FMT "@%li%n:%li:%li%n" > #endif A more typical approach is akin to ; have PH_ADDR_FMT defined to either "lli" or "li", then write sscanf(str, "@%"PH_ADDR_FMT "%n:..., ...), using PH_ADDR_FMT multiple times. > ... > processed = sscanf(str, PH_ADDR_SCAN_FMT, >, , >_off, _off, ); Umm, why are you passing to more than one sscanf() %? That's (probably) undefined behavior. [In general, sscanf() is a horrid interface to use for parsing integers - it has undefined behavior if the input text would trigger integer overflow, making it safe to use ONLY on text that you control and can guarantee won't overflow. By the time you've figured out if untrusted text meets the requirement for safe parsing via sscanf(), you've practically already parsed it via safer strtol() and friends.] -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v5 1/4] firmware: introduce sysfs driver for QEMU's fw_cfg device
On 11/24/2015 09:55 AM, Gabriel L. Somlo wrote: > On Tue, Nov 24, 2015 at 04:14:50AM +0800, kbuild test robot wrote: >> >>drivers/firmware/qemu_fw_cfg.c: In function 'fw_cfg_cmdline_set': >>>> drivers/firmware/qemu_fw_cfg.c:510:7: warning: format '%lli' expects >>>> argument of type 'long long int *', but argument 3 has type 'phys_addr_t >>>> *' [-Wformat=] >> _off, _off, ); >> ^ > > Oh, I think I know why this happened: > > > So, I could use u64 instead of phys_addr_t and resource_size_t, and > keep "%lli" (or "%Li"), but then I'd have to check if the parsed value %Li is not POSIX. Don't use it (stick with %lli). > would overflow a 32-bit address value on arches where phys_addr_t is > u32, which would make things a bit more messy and awkward. > > I'm planning on #ifdef-ing the format string instead: > > #ifdef CONFIG_PHYS_ADDR_T_64BIT > #define PH_ADDR_SCAN_FMT "@%Li%n:%Li:%Li%n" > #else > #define PH_ADDR_SCAN_FMT "@%li%n:%li:%li%n" > #endif A more typical approach is akin to ; have PH_ADDR_FMT defined to either "lli" or "li", then write sscanf(str, "@%"PH_ADDR_FMT "%n:..., ...), using PH_ADDR_FMT multiple times. > ... > processed = sscanf(str, PH_ADDR_SCAN_FMT, >, , >_off, _off, ); Umm, why are you passing to more than one sscanf() %? That's (probably) undefined behavior. [In general, sscanf() is a horrid interface to use for parsing integers - it has undefined behavior if the input text would trigger integer overflow, making it safe to use ONLY on text that you control and can guarantee won't overflow. By the time you've figured out if untrusted text meets the requirement for safe parsing via sscanf(), you've practically already parsed it via safer strtol() and friends.] -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] QEMU fw_cfg DMA interface
On 10/01/2015 10:17 AM, Laszlo Ersek wrote: > On 10/01/15 18:03, Eric Blake wrote: >> [meta-comment] >> >> On 10/01/2015 06:14 AM, Marc Marí wrote: >>> Implementation of the FW CFG DMA interface. >> >> The subject line is missing "v4" and "0/7". Also, the cover letter is >> missing a diffstat. That makes it harder to see from the cover letter >> what the rest of the series is about. 'git format-patch/send-email >> --cover-letter' does what you want; you can even 'git config >> format.coverletter=auto' to always include a decent cover letter on any >> multi-patch series. >> > > This posting follows a little bit different pattern, one that I myself > follow when posting patches for two (or more) components that must work > in sync. Ok, makes sense. Maybe the only additional suggestions would be to make it more obvious in the subject line (put the text 'cross-post' somewhere?) or have the first paragraph of the meta-cover be more explicit that there are going to be multiple sub-threads, one per project, where all subthreads must be applied to their corresponding project for the overall feature to be complete? [And maybe I should wait a few minutes for the full thread to appear in my inbox, rather than immediately replying to the first mail while the series is still incomplete due to mail delays...] -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] QEMU fw_cfg DMA interface
On 10/01/2015 10:03 AM, Eric Blake wrote: > [meta-comment] > > On 10/01/2015 06:14 AM, Marc Marí wrote: >> Implementation of the FW CFG DMA interface. > > The subject line is missing "v4" and "0/7". Also, the cover letter is > missing a diffstat. That makes it harder to see from the cover letter > what the rest of the series is about. 'git format-patch/send-email > --cover-letter' does what you want; you can even 'git config > format.coverletter=auto' to always include a decent cover letter on any > multi-patch series. Oh, I see - you sent a meta-cover letter (the one I replied to in this subthread), and then a patch series including a cover letter (the real 0/7, then 1/7 and friends in-reply-to the 0/7) as a child of the meta-cover. It's still a bit awkward for tools that expect the 0/7 as the start of the thread, and part of my confusion was caused by out-of-order mail delivery due to the nongnu.org mail server still recovering from its mail delays. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] QEMU fw_cfg DMA interface
[meta-comment] On 10/01/2015 06:14 AM, Marc Marí wrote: > Implementation of the FW CFG DMA interface. The subject line is missing "v4" and "0/7". Also, the cover letter is missing a diffstat. That makes it harder to see from the cover letter what the rest of the series is about. 'git format-patch/send-email --cover-letter' does what you want; you can even 'git config format.coverletter=auto' to always include a decent cover letter on any multi-patch series. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] QEMU fw_cfg DMA interface
On 10/01/2015 10:03 AM, Eric Blake wrote: > [meta-comment] > > On 10/01/2015 06:14 AM, Marc Marí wrote: >> Implementation of the FW CFG DMA interface. > > The subject line is missing "v4" and "0/7". Also, the cover letter is > missing a diffstat. That makes it harder to see from the cover letter > what the rest of the series is about. 'git format-patch/send-email > --cover-letter' does what you want; you can even 'git config > format.coverletter=auto' to always include a decent cover letter on any > multi-patch series. Oh, I see - you sent a meta-cover letter (the one I replied to in this subthread), and then a patch series including a cover letter (the real 0/7, then 1/7 and friends in-reply-to the 0/7) as a child of the meta-cover. It's still a bit awkward for tools that expect the 0/7 as the start of the thread, and part of my confusion was caused by out-of-order mail delivery due to the nongnu.org mail server still recovering from its mail delays. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] QEMU fw_cfg DMA interface
On 10/01/2015 10:17 AM, Laszlo Ersek wrote: > On 10/01/15 18:03, Eric Blake wrote: >> [meta-comment] >> >> On 10/01/2015 06:14 AM, Marc Marí wrote: >>> Implementation of the FW CFG DMA interface. >> >> The subject line is missing "v4" and "0/7". Also, the cover letter is >> missing a diffstat. That makes it harder to see from the cover letter >> what the rest of the series is about. 'git format-patch/send-email >> --cover-letter' does what you want; you can even 'git config >> format.coverletter=auto' to always include a decent cover letter on any >> multi-patch series. >> > > This posting follows a little bit different pattern, one that I myself > follow when posting patches for two (or more) components that must work > in sync. Ok, makes sense. Maybe the only additional suggestions would be to make it more obvious in the subject line (put the text 'cross-post' somewhere?) or have the first paragraph of the meta-cover be more explicit that there are going to be multiple sub-threads, one per project, where all subthreads must be applied to their corresponding project for the overall feature to be complete? [And maybe I should wait a few minutes for the full thread to appear in my inbox, rather than immediately replying to the first mail while the series is still incomplete due to mail delays...] -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] QEMU fw_cfg DMA interface
[meta-comment] On 10/01/2015 06:14 AM, Marc Marí wrote: > Implementation of the FW CFG DMA interface. The subject line is missing "v4" and "0/7". Also, the cover letter is missing a diffstat. That makes it harder to see from the cover letter what the rest of the series is about. 'git format-patch/send-email --cover-letter' does what you want; you can even 'git config format.coverletter=auto' to always include a decent cover letter on any multi-patch series. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [PATCH v4 12/15] Add optional parameters to QMP command query-cpu-definitions
On 03/31/2015 01:46 PM, Eduardo Habkost wrote:
> On Mon, Mar 30, 2015 at 04:28:25PM +0200, Michael Mueller wrote:
> [...]
>> ##
>> # @query-cpu-definitions:
>> #
>> # Return a list of supported virtual CPU definitions
>> #
>> +# @machine: #optional machine type (since 2.4)
>> +#
>> +# @accel: #optional accelerator id (since 2.4)
>> +#
>> # Returns: a list of CpuDefInfo
>> #
>> # Since: 1.2.0
>> ##
>> -{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] }
>> +{ 'command': 'query-cpu-definitions',
>> + 'data': { '*machine': 'str', '*accel': 'AccelId' },
>> + 'returns': ['CpuDefinitionInfo'] }
>
> What happens if the new parameters are provided to an old QEMU version
> that doesn't accept them? It looks like we need an introspection
> mechanism or a new command name.
Providing an optional parameter that a new qemu understands to an older
qemu gracefully errors out about an unknown parameter. But it's
annoying to have to probe for whether the parameter is understood by
exploiting that particular error message from older qemu.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v4 12/15] Add optional parameters to QMP command query-cpu-definitions
On 03/31/2015 01:46 PM, Eduardo Habkost wrote:
On Mon, Mar 30, 2015 at 04:28:25PM +0200, Michael Mueller wrote:
[...]
##
# @query-cpu-definitions:
#
# Return a list of supported virtual CPU definitions
#
+# @machine: #optional machine type (since 2.4)
+#
+# @accel: #optional accelerator id (since 2.4)
+#
# Returns: a list of CpuDefInfo
#
# Since: 1.2.0
##
-{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] }
+{ 'command': 'query-cpu-definitions',
+ 'data': { '*machine': 'str', '*accel': 'AccelId' },
+ 'returns': ['CpuDefinitionInfo'] }
What happens if the new parameters are provided to an old QEMU version
that doesn't accept them? It looks like we need an introspection
mechanism or a new command name.
Providing an optional parameter that a new qemu understands to an older
qemu gracefully errors out about an unknown parameter. But it's
annoying to have to probe for whether the parameter is understood by
exploiting that particular error message from older qemu.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v4 12/15] Add optional parameters to QMP command query-cpu-definitions
On 03/30/2015 08:28 AM, Michael Mueller wrote:
> The patch adds optional parameters to the QMP command query-cpu-definitions.
> Thus the signature of routine arch_query_cpu_definitions needs to be changed
> for the stub function and all target implementations:
>
> target-arm
> target-i386
> target-ppc
> target-s390
>
> Signed-off-by: Michael Mueller
> ---
> +++ b/qapi-schema.json
> @@ -2532,21 +2532,31 @@
> #
> # @name: the name of the CPU definition
> #
> +# @default: #optional defines if cpu model is the default (since 2.4)
Reads poorly. How about:
# @default: #optional true if cpu model is the default, omitted if false
(since 2.4)
> +#
> +# @runnable: #optional defines if cpu model is runnable (since 2.4)
Similarly:
# @runnable: #optional true if cpu model is runnable, omitted if false
(since 2.4)
> +#
> # Since: 1.2.0
> ##
> { 'type': 'CpuDefinitionInfo',
> - 'data': { 'name': 'str' } }
> + 'data': { 'name': 'str', '*is-default': 'bool', '*runnable': 'bool' } }
>
> ##
> # @query-cpu-definitions:
> #
> # Return a list of supported virtual CPU definitions
> #
> +# @machine: #optional machine type (since 2.4)
> +#
> +# @accel: #optional accelerator id (since 2.4)
Maybe mention that these two fields are for filtering results.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
On 03/30/2015 02:17 PM, Eduardo Habkost wrote:
> On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote:
>> This patch implements a new QMP request named 'query-cpu-model'.
>> It returns the cpu model of cpu 0 and its backing accelerator.
>>
>> request:
>> {"execute" : "query-cpu-model" }
>>
>> answer:
>> {"return" : {"name": "2827-ga2", "accel": "kvm" }}
>
> If you are returning information about an existing CPU, why not just
> extend the output of "query-cpus"?
>
> (Existing qmp_query_cpus() calls cpu_synchronize_state(), which may be
> undesired. But in this case we could add an optional parameter to
> disable the return of data that requires stopping the VCPU).
And how would libvirt learn about the existence of that optional
parameter? Without introspection, a new command is easier to query than
learning about whether an optional parameter exists (then again, we're
hoping to get introspection into 2.4, so it may be a moot question).
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
On 03/30/2015 08:28 AM, Michael Mueller wrote:
> This patch implements a new QMP request named 'query-cpu-model'.
> It returns the cpu model of cpu 0 and its backing accelerator.
>
> request:
> {"execute" : "query-cpu-model" }
>
> answer:
> {"return" : {"name": "2827-ga2", "accel": "kvm" }}
>
> Alias names are resolved to their respective machine type and GA names
> already during cpu instantiation. Thus, also a cpu model like 'host'
> which is implemented as alias will return its normalized cpu model name.
>
> Furthermore the patch implements the following function:
>
> - s390_cpu_models_used(), returns true if S390 cpu models are in use
>
> Signed-off-by: Michael Mueller
> ---
> +++ b/qapi-schema.json
> @@ -2516,6 +2516,16 @@
> { 'command': 'query-machines', 'returns': ['MachineInfo'] }
>
> ##
> +# @AccelId
> +#
> +# Defines accelerator ids
> +#
> +# Since: 2.4
> +##
> +{ 'enum': 'AccelId',
> + 'data': ['qtest', 'tcg', 'kvm', 'xen' ] }
Unusual spacing (0 spaces after '[' but 2 spaces before closing ']'?),
but not necessarily wrong.
> +##
> +# @CpuModelInfo:
> +#
> +# Virtual CPU model definition.
> +#
> +# @name: the name of the CPU model definition
> +#
> +# @accel: AccelId (name) of this cpu models accelerator
s/models/model's/
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v4 12/15] Add optional parameters to QMP command query-cpu-definitions
On 03/30/2015 08:28 AM, Michael Mueller wrote:
The patch adds optional parameters to the QMP command query-cpu-definitions.
Thus the signature of routine arch_query_cpu_definitions needs to be changed
for the stub function and all target implementations:
target-arm
target-i386
target-ppc
target-s390
Signed-off-by: Michael Mueller m...@linux.vnet.ibm.com
---
+++ b/qapi-schema.json
@@ -2532,21 +2532,31 @@
#
# @name: the name of the CPU definition
#
+# @default: #optional defines if cpu model is the default (since 2.4)
Reads poorly. How about:
# @default: #optional true if cpu model is the default, omitted if false
(since 2.4)
+#
+# @runnable: #optional defines if cpu model is runnable (since 2.4)
Similarly:
# @runnable: #optional true if cpu model is runnable, omitted if false
(since 2.4)
+#
# Since: 1.2.0
##
{ 'type': 'CpuDefinitionInfo',
- 'data': { 'name': 'str' } }
+ 'data': { 'name': 'str', '*is-default': 'bool', '*runnable': 'bool' } }
##
# @query-cpu-definitions:
#
# Return a list of supported virtual CPU definitions
#
+# @machine: #optional machine type (since 2.4)
+#
+# @accel: #optional accelerator id (since 2.4)
Maybe mention that these two fields are for filtering results.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
On 03/30/2015 02:17 PM, Eduardo Habkost wrote:
On Mon, Mar 30, 2015 at 04:28:24PM +0200, Michael Mueller wrote:
This patch implements a new QMP request named 'query-cpu-model'.
It returns the cpu model of cpu 0 and its backing accelerator.
request:
{execute : query-cpu-model }
answer:
{return : {name: 2827-ga2, accel: kvm }}
If you are returning information about an existing CPU, why not just
extend the output of query-cpus?
(Existing qmp_query_cpus() calls cpu_synchronize_state(), which may be
undesired. But in this case we could add an optional parameter to
disable the return of data that requires stopping the VCPU).
And how would libvirt learn about the existence of that optional
parameter? Without introspection, a new command is easier to query than
learning about whether an optional parameter exists (then again, we're
hoping to get introspection into 2.4, so it may be a moot question).
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v4 11/15] target-s390x: New QMP command query-cpu-model
On 03/30/2015 08:28 AM, Michael Mueller wrote:
This patch implements a new QMP request named 'query-cpu-model'.
It returns the cpu model of cpu 0 and its backing accelerator.
request:
{execute : query-cpu-model }
answer:
{return : {name: 2827-ga2, accel: kvm }}
Alias names are resolved to their respective machine type and GA names
already during cpu instantiation. Thus, also a cpu model like 'host'
which is implemented as alias will return its normalized cpu model name.
Furthermore the patch implements the following function:
- s390_cpu_models_used(), returns true if S390 cpu models are in use
Signed-off-by: Michael Mueller m...@linux.vnet.ibm.com
---
+++ b/qapi-schema.json
@@ -2516,6 +2516,16 @@
{ 'command': 'query-machines', 'returns': ['MachineInfo'] }
##
+# @AccelId
+#
+# Defines accelerator ids
+#
+# Since: 2.4
+##
+{ 'enum': 'AccelId',
+ 'data': ['qtest', 'tcg', 'kvm', 'xen' ] }
Unusual spacing (0 spaces after '[' but 2 spaces before closing ']'?),
but not necessarily wrong.
+##
+# @CpuModelInfo:
+#
+# Virtual CPU model definition.
+#
+# @name: the name of the CPU model definition
+#
+# @accel: AccelId (name) of this cpu models accelerator
s/models/model's/
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 02/21] userfaultfd: linux/Documentation/vm/userfaultfd.txt
d thread to wake userfaults that > +haven't been read by userland yet. If we would do that likely the > +UFFDIO_WAKE ioctl could be dropped. This may change in the future > +(with a UFFD_API protocol bumb combined with the removal of the s/bumb/bump/ > +UFFDIO_WAKE ioctl) if it'll be demonstrated that it's a valid > +optimization and worthy to force userland to use the UFFD always in > +nonblocking mode if combined with POLLIN. > + > +userfaultfd is also a generic enough feature, that it allows KVM to > +implement postcopy live migration (one form of memory externalization > +consisting of a virtual machine running with part or all of its memory > +residing on a different node in the cloud) without having to modify a > +single line of KVM kernel code. Guest async page faults, FOLL_NOWAIT > +and all other GUP features works just fine in combination with > +userfaults (userfaults trigger async page faults in the guest > +scheduler so those guest processes that aren't waiting for userfaults > +can keep running in the guest vcpus). > + > +The primary ioctl to resolve userfaults is UFFDIO_COPY. That > +atomically copies a page into the userfault registered range and wakes > +up the blocked userfaults (unless uffdio_copy.mode & > +UFFDIO_COPY_MODE_DONTWAKE is set). Other ioctl works similarly to > +UFFDIO_COPY. > > > -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 02/21] userfaultfd: linux/Documentation/vm/userfaultfd.txt
userland to use the UFFD always in +nonblocking mode if combined with POLLIN. + +userfaultfd is also a generic enough feature, that it allows KVM to +implement postcopy live migration (one form of memory externalization +consisting of a virtual machine running with part or all of its memory +residing on a different node in the cloud) without having to modify a +single line of KVM kernel code. Guest async page faults, FOLL_NOWAIT +and all other GUP features works just fine in combination with +userfaults (userfaults trigger async page faults in the guest +scheduler so those guest processes that aren't waiting for userfaults +can keep running in the guest vcpus). + +The primary ioctl to resolve userfaults is UFFDIO_COPY. That +atomically copies a page into the userfault registered range and wakes +up the blocked userfaults (unless uffdio_copy.mode +UFFDIO_COPY_MODE_DONTWAKE is set). Other ioctl works similarly to +UFFDIO_COPY. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [RFC PATCH v2 02/15] cpu-model: Introduce option --probe to switch into probe mode
On 02/17/2015 07:24 AM, Michael Mueller wrote: > The option --probe allows to switch into probe mode also for machines > different from none. If one or more accelerators are specified these > accelerators are used to provide probable properties. If no accelerator > is given a list of accellerators that support probing is used. s/accellerators/accelerators/ > > Signed-off-by: Michael Mueller > --- > accel.c| 13 - > include/sysemu/accel.h | 2 +- > qemu-options.hx| 8 > vl.c | 7 ++- > 4 files changed, 23 insertions(+), 7 deletions(-) > -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [RFC PATCH v2 12/15] cpu-model/s390: Extend QMP command query-cpu-definitions
On 02/17/2015 07:24 AM, Michael Mueller wrote:
> This patch implements the QMP command 'query-cpu-definitions' in the S390
> context. The command returns a in terms of machine release date descending
> sorted list of cpu model names in the current host context.
returns a list of cpu model names sorted by descending release dates.
Does guaranteeing the sorting as part of the interface really matter, or
would it be better to just return the list with no documented sorting
(where callers treat it as unsorted)?
> A consumer may
> successfully request each listed cpu model as long for a given accelerator
> this model is runnable.
>
> Thy QMP type AccelCpuModelInfo is introduced and the type CpuDefinitionInfo
> is extended by the optional field 'accelerators'. It contains a list of named
> accelerators and some indication whether the associated cpu model is runnable
> or the default cpu model. The default cpu model is used either no specific cpu
> was requested during QEMU startup or the cpu model with named 'host'.
>
> request:
> {"execute": "query-cpu-definitions"}
>
> answer:
> {"return":
>
> [{"name":"2964-ga1","accelerators":[{"name":"kvm","runnable":false,"default":false}]},
>
> {"name":"2828-ga1","accelerators":[{"name":"kvm","runnable":false,"default":false}]},
>
> {"name":"2827-ga2","accelerators":[{"name":"kvm","runnable":true,"default":true}]},
>
> {"name":"2827-ga1","accelerators":[{"name":"kvm","runnable":true,"default":false}]},
>
> {"name":"2818-ga1","accelerators":[{"name":"kvm","runnable":true,"default":false}]},
> ...
>
> {"name":"2064-ga1","accelerators":[{"runnable":true,"name":"kvm","default":false}]}
> ]
>}
>
Looks okay from an interface perspective.
> Signed-off-by: Michael Mueller
> ---
> qapi-schema.json | 21 +-
> target-s390x/cpu-models.c | 15 +++
> target-s390x/cpu-models.h | 1 +
> target-s390x/cpu.c| 100
> +++---
> 4 files changed, 130 insertions(+), 7 deletions(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 9431fc2..a5d38ae 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2485,16 +2485,35 @@
>'data': ['qtest', 'tcg', 'kvm', 'xen' ] }
>
> ##
> +# @AccelCpuModelInfo:
> +#
> +# Accelerator specific CPU model data
> +#
> +# @id: the accelerator id
> +#
There is no 'id' field below, did you mean 'name'?
> +# @default: cpu model for 'host'
> +#
> +# @runnable: cpu model can be activated on hosting machine
> +#
> +# Since: 2.3.0
> +#
> +##
> +{ 'type': 'AccelCpuModelInfo',
> + 'data': { 'name': 'AccelId', 'default': 'bool', 'runnable': 'bool' } }
> +
> +##
> # @CpuDefinitionInfo:
> #
> # Virtual CPU definition.
> #
> # @name: the name of the CPU definition
> #
> +# @accelerators: #optional cpu model offered per accelerator (since 2.3.0)
> +#
Must the field be optional, or will we always provide it? Since this is
an output-only field, it is okay for back-compat to make the new field
unconditional.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [RFC PATCH v2 11/15] cpu-model/s390: Add QMP command query-cpu-model
On 02/17/2015 07:24 AM, Michael Mueller wrote:
> This patch implements a new QMP request named 'query-cpu-model'.
> It returns the cpu model of cpu 0 and its backing accelerator.
>
> request:
> {"execute" : "query-cpu-model" }
>
> answer:
> {"return" : {"name": "2827-ga2", "accelerator": "kvm" }}
>
> Alias names are resolved to their respective machine type and GA names
> already during cpu instantiation. Thus, also a cpu model like 'host'
> which is implemented as alias will return its normalized cpu model name.
>
> Furthermore the patch implements the following functions:
>
> - s390_cpu_typename(), returns the currently selected cpu type name or NULL
> - s390_cpu_models_used(), returns true if S390 cpu models are in use
>
> Signed-off-by: Michael Mueller
> ---
>
> +##
> +# @CpuModelInfo:
> +#
> +# Virtual CPU model definition.
> +#
> +# @name: the name of the CPU model definition
> +#
> +# Since: 2.3.0
> +##
> +{ 'type': 'CpuModelInfo',
> + 'data': { 'name': 'str', '*accelerator': 'AccelId' } }
You didn't document '*accelerator', including mention that it is
optional (why would it not be output always?).
> +
> +##
> +# @query-cpu-model:
> +#
> +# Return to current virtual CPU model
s/to/the/
> +#
> +# Returns: CpuModelInfo
> +#
> +# Since: 2.3.0
We aren't very consistent on '2.3' vs. '2.3.0', so I won't complain
about that.
> +##
> +{ 'command': 'query-cpu-model', 'returns': 'CpuModelInfo' }
Seems reasonable from the interface point of view; I have not closely
reviewed the implementation.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [RFC PATCH v2 12/15] cpu-model/s390: Extend QMP command query-cpu-definitions
On 02/17/2015 07:24 AM, Michael Mueller wrote:
This patch implements the QMP command 'query-cpu-definitions' in the S390
context. The command returns a in terms of machine release date descending
sorted list of cpu model names in the current host context.
returns a list of cpu model names sorted by descending release dates.
Does guaranteeing the sorting as part of the interface really matter, or
would it be better to just return the list with no documented sorting
(where callers treat it as unsorted)?
A consumer may
successfully request each listed cpu model as long for a given accelerator
this model is runnable.
Thy QMP type AccelCpuModelInfo is introduced and the type CpuDefinitionInfo
is extended by the optional field 'accelerators'. It contains a list of named
accelerators and some indication whether the associated cpu model is runnable
or the default cpu model. The default cpu model is used either no specific cpu
was requested during QEMU startup or the cpu model with named 'host'.
request:
{execute: query-cpu-definitions}
answer:
{return:
[{name:2964-ga1,accelerators:[{name:kvm,runnable:false,default:false}]},
{name:2828-ga1,accelerators:[{name:kvm,runnable:false,default:false}]},
{name:2827-ga2,accelerators:[{name:kvm,runnable:true,default:true}]},
{name:2827-ga1,accelerators:[{name:kvm,runnable:true,default:false}]},
{name:2818-ga1,accelerators:[{name:kvm,runnable:true,default:false}]},
...
{name:2064-ga1,accelerators:[{runnable:true,name:kvm,default:false}]}
]
}
Looks okay from an interface perspective.
Signed-off-by: Michael Mueller m...@linux.vnet.ibm.com
---
qapi-schema.json | 21 +-
target-s390x/cpu-models.c | 15 +++
target-s390x/cpu-models.h | 1 +
target-s390x/cpu.c| 100
+++---
4 files changed, 130 insertions(+), 7 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 9431fc2..a5d38ae 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2485,16 +2485,35 @@
'data': ['qtest', 'tcg', 'kvm', 'xen' ] }
##
+# @AccelCpuModelInfo:
+#
+# Accelerator specific CPU model data
+#
+# @id: the accelerator id
+#
There is no 'id' field below, did you mean 'name'?
+# @default: cpu model for 'host'
+#
+# @runnable: cpu model can be activated on hosting machine
+#
+# Since: 2.3.0
+#
+##
+{ 'type': 'AccelCpuModelInfo',
+ 'data': { 'name': 'AccelId', 'default': 'bool', 'runnable': 'bool' } }
+
+##
# @CpuDefinitionInfo:
#
# Virtual CPU definition.
#
# @name: the name of the CPU definition
#
+# @accelerators: #optional cpu model offered per accelerator (since 2.3.0)
+#
Must the field be optional, or will we always provide it? Since this is
an output-only field, it is okay for back-compat to make the new field
unconditional.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [RFC PATCH v2 11/15] cpu-model/s390: Add QMP command query-cpu-model
On 02/17/2015 07:24 AM, Michael Mueller wrote:
This patch implements a new QMP request named 'query-cpu-model'.
It returns the cpu model of cpu 0 and its backing accelerator.
request:
{execute : query-cpu-model }
answer:
{return : {name: 2827-ga2, accelerator: kvm }}
Alias names are resolved to their respective machine type and GA names
already during cpu instantiation. Thus, also a cpu model like 'host'
which is implemented as alias will return its normalized cpu model name.
Furthermore the patch implements the following functions:
- s390_cpu_typename(), returns the currently selected cpu type name or NULL
- s390_cpu_models_used(), returns true if S390 cpu models are in use
Signed-off-by: Michael Mueller m...@linux.vnet.ibm.com
---
+##
+# @CpuModelInfo:
+#
+# Virtual CPU model definition.
+#
+# @name: the name of the CPU model definition
+#
+# Since: 2.3.0
+##
+{ 'type': 'CpuModelInfo',
+ 'data': { 'name': 'str', '*accelerator': 'AccelId' } }
You didn't document '*accelerator', including mention that it is
optional (why would it not be output always?).
+
+##
+# @query-cpu-model:
+#
+# Return to current virtual CPU model
s/to/the/
+#
+# Returns: CpuModelInfo
+#
+# Since: 2.3.0
We aren't very consistent on '2.3' vs. '2.3.0', so I won't complain
about that.
+##
+{ 'command': 'query-cpu-model', 'returns': 'CpuModelInfo' }
Seems reasonable from the interface point of view; I have not closely
reviewed the implementation.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [RFC PATCH v2 02/15] cpu-model: Introduce option --probe to switch into probe mode
On 02/17/2015 07:24 AM, Michael Mueller wrote: The option --probe allows to switch into probe mode also for machines different from none. If one or more accelerators are specified these accelerators are used to provide probable properties. If no accelerator is given a list of accellerators that support probing is used. s/accellerators/accelerators/ Signed-off-by: Michael Mueller m...@linux.vnet.ibm.com --- accel.c| 13 - include/sysemu/accel.h | 2 +- qemu-options.hx| 8 vl.c | 7 ++- 4 files changed, 23 insertions(+), 7 deletions(-) -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v1 RFC 09/10] QEMU: s390: cpu model QMP query-cpu-model
On 05/13/2014 09:00 AM, Michael Mueller wrote:
> This patch implements a new QMP request named "query-cpu-model". It returns
> the cpu model of cpu 0. eg:
>
> request: '{"execute" : "query-cpu-model" }
> answer: '{"return" : {"name": "2827-ga2"}}
>
> Alias names are resolved to their respective machine type and GA names
> already during cpu instantiation. Thus, also a cpu name like "host",
> which is implemented as alias, will return its normalized cpu model name.
>
> +}
> +cpu_model = g_try_malloc0(sizeof(*cpu_model));
It's simpler to just use g_malloc0 and rely on glibc's exit-on-OOM
behavior than to try and deal with NULL - this isn't user input (so
unlikely to be so huge as to cause OOM), and would be more in line with
what most other QMP code does. But that said...
> +if (!cpu_model) {
> +goto out_no_mem;
> +}
> +cpu_model->name = g_try_malloc0(CPU_MODEL_NAME_LEN);
> +if (!cpu_model->name) {
> +goto out_no_mem;
> +}
> +snprintf(cpu_model->name, CPU_MODEL_NAME_LEN - 1, "%04x-ga%u",
> + cc->proc->type, cc->mach->ga);
...why not just use g_strdup_printf() instead of trying to size a buffer
yourself? In other words, skip the g_try_malloc0 to begin with.
The fact that you are packing two pieces of information into one string
is a bit worrisome - that means that the client of the QMP command has
to parse the string back into two pieces of information if they ever
need either item in isolation. If the user never has a need to split
the name down into parts, you are okay; I don't know S390 well enough to
know whether anyone will care about type separate from ga. But if
someone DOES care, then the QMP command should return the parts already
split, as in { "type": 2827, "ga": 2 }, or even as convenience provide
both split and combined forms: { "name": "2827-ga2", "type": 2827, "ga": 2 }
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v1 RFC 09/10] QEMU: s390: cpu model QMP query-cpu-model
On 05/13/2014 09:00 AM, Michael Mueller wrote:
This patch implements a new QMP request named query-cpu-model. It returns
the cpu model of cpu 0. eg:
request: '{execute : query-cpu-model }
answer: '{return : {name: 2827-ga2}}
Alias names are resolved to their respective machine type and GA names
already during cpu instantiation. Thus, also a cpu name like host,
which is implemented as alias, will return its normalized cpu model name.
+}
+cpu_model = g_try_malloc0(sizeof(*cpu_model));
It's simpler to just use g_malloc0 and rely on glibc's exit-on-OOM
behavior than to try and deal with NULL - this isn't user input (so
unlikely to be so huge as to cause OOM), and would be more in line with
what most other QMP code does. But that said...
+if (!cpu_model) {
+goto out_no_mem;
+}
+cpu_model-name = g_try_malloc0(CPU_MODEL_NAME_LEN);
+if (!cpu_model-name) {
+goto out_no_mem;
+}
+snprintf(cpu_model-name, CPU_MODEL_NAME_LEN - 1, %04x-ga%u,
+ cc-proc-type, cc-mach-ga);
...why not just use g_strdup_printf() instead of trying to size a buffer
yourself? In other words, skip the g_try_malloc0 to begin with.
The fact that you are packing two pieces of information into one string
is a bit worrisome - that means that the client of the QMP command has
to parse the string back into two pieces of information if they ever
need either item in isolation. If the user never has a need to split
the name down into parts, you are okay; I don't know S390 well enough to
know whether anyone will care about type separate from ga. But if
someone DOES care, then the QMP command should return the parts already
split, as in { type: 2827, ga: 2 }, or even as convenience provide
both split and combined forms: { name: 2827-ga2, type: 2827, ga: 2 }
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: RFC: allow empty symlink targets
mentioned above does not change that fact, it only changed the standard to have a more sensible requirement on what happens when dereferencing an empty symlink. If you would like to file a bug against POSIX requesting that the next version of POSIX permit implementations to reject an empty path1 in the symlink() call on the grounds that you are unable to fix the Linux kernel symlink() behavior to comply with existing POSIX wording, be my guest. The bug process is open, and anyone can file a bug, although I don't know that I want to do it on your behalf. Here's where you would do it: http://austingroupbugs.net/ At the end of the day, I personally don't care whether you fix the Linux kernel symlink() to allow empty symlinks, or successfully argue for a bug fix against POSIX to permit the existing Linux symlink() behavior. I'd love to see for Linux obtain POSIX certification someday, and either of those two courses of action would get us closer. Meanwhile, I know there are enough other issues in the kernel (implementing O_SEARCH, for example) that it will be a long time before we ever get a POSIX certification of a Linux system. And as long as Linux doesn't have POSIX certification, corner cases like non-compliance with a requirement that has been around for more than 12 years won't make life any harder for people that already have to be aware that not all the world is POSIX. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: RFC: allow empty symlink targets
of POSIX permit implementations to reject an empty path1 in the symlink() call on the grounds that you are unable to fix the Linux kernel symlink() behavior to comply with existing POSIX wording, be my guest. The bug process is open, and anyone can file a bug, although I don't know that I want to do it on your behalf. Here's where you would do it: http://austingroupbugs.net/ At the end of the day, I personally don't care whether you fix the Linux kernel symlink() to allow empty symlinks, or successfully argue for a bug fix against POSIX to permit the existing Linux symlink() behavior. I'd love to see for Linux obtain POSIX certification someday, and either of those two courses of action would get us closer. Meanwhile, I know there are enough other issues in the kernel (implementing O_SEARCH, for example) that it will be a long time before we ever get a POSIX certification of a Linux system. And as long as Linux doesn't have POSIX certification, corner cases like non-compliance with a requirement that has been around for more than 12 years won't make life any harder for people that already have to be aware that not all the world is POSIX. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: RFC: allow empty symlink targets
On 05/15/2013 06:38 AM, Pádraig Brady wrote: > On 01/17/2013 04:22 PM, Pádraig Brady wrote: >> On 01/17/2013 01:03 PM, Pádraig Brady wrote: >>> The discussion leading to this is at http://bugs.gnu.org/13447 >>> In summary other systems allow an empty target for a symlink, >>> and POSIX specifies that it should be allowed? >> >> In relation to this, Eric Blake said: >> >>> In today's Austin Group meeting, I was tasked to open a new bug that >>> would state specifically how the empty symlink is resolved; the intent >>> is to allow both Solaris behavior (current directory) and BSD behavior >>> (ENOENT). Meanwhile, everyone was in agreement that the Linux kernel >>> has a bug for rejecting the creation of an empty symlink, but once that >>> bug is fixed, then Linux can choose either Solaris or BSD behavior for >>> how to resolve such a symlink. >>> >>> It will probably be a bug report similar to this one, which regarded how >>> to handle a symlink containing just slashes: >>> http://austingroupbugs.net/view.php?id=541 > > Following up from http://austingroupbugs.net/view.php?id=649 > It seems POSIX will now allow the current Linux behavior of returning ENOENT, Huh? Linux currently doesn't allow the creation of an empty symlink. That link mentions the current BSD behavior of returning ENOENT when resolving such a symlink (that is, what stat() does when chasing through an empty symlink, provided such a symlink is first created). > or the Solaris behavior of allowing empty symlink targets. The point made in that bug report is that Linux is buggy for not allowing symlink() to create an empty symlink in the first place; once you allow the creation of an empty symlink, then how to handle such a symlink in stat() is up to you whether to copy Solaris' or BSD's example. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: RFC: allow empty symlink targets
On 05/15/2013 06:38 AM, Pádraig Brady wrote: On 01/17/2013 04:22 PM, Pádraig Brady wrote: On 01/17/2013 01:03 PM, Pádraig Brady wrote: The discussion leading to this is at http://bugs.gnu.org/13447 In summary other systems allow an empty target for a symlink, and POSIX specifies that it should be allowed? In relation to this, Eric Blake said: In today's Austin Group meeting, I was tasked to open a new bug that would state specifically how the empty symlink is resolved; the intent is to allow both Solaris behavior (current directory) and BSD behavior (ENOENT). Meanwhile, everyone was in agreement that the Linux kernel has a bug for rejecting the creation of an empty symlink, but once that bug is fixed, then Linux can choose either Solaris or BSD behavior for how to resolve such a symlink. It will probably be a bug report similar to this one, which regarded how to handle a symlink containing just slashes: http://austingroupbugs.net/view.php?id=541 Following up from http://austingroupbugs.net/view.php?id=649 It seems POSIX will now allow the current Linux behavior of returning ENOENT, Huh? Linux currently doesn't allow the creation of an empty symlink. That link mentions the current BSD behavior of returning ENOENT when resolving such a symlink (that is, what stat() does when chasing through an empty symlink, provided such a symlink is first created). or the Solaris behavior of allowing empty symlink targets. The point made in that bug report is that Linux is buggy for not allowing symlink() to create an empty symlink in the first place; once you allow the creation of an empty symlink, then how to handle such a symlink in stat() is up to you whether to copy Solaris' or BSD's example. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [libvirt] if_bridge.h: include in6.h for struct in6_addr use
On 03/13/2013 09:17 AM, Kumar Gala wrote: >>>> >>>> if_bridge.h uses struct in6_addr ip6; but does not include the in6.h >>>> header. >>>> >>>> Found by trying to build libvirt and connman against 3.8-rc3 headers. >>>> >>> > Did this ever get resolved ? Libvirt has managed to work around the kernel issue in the meantime. But just today, I hit the same issue with the latest kernel-headers-3.8.2-206.fc18.x86_64 on Fedora 18 when backporting to an older libvirt branch that did not have the workaround. A kernel person would have to speak up for certain, but I think it is still a problem. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [libvirt] if_bridge.h: include in6.h for struct in6_addr use
On 03/13/2013 09:17 AM, Kumar Gala wrote: if_bridge.h uses struct in6_addr ip6; but does not include the in6.h header. Found by trying to build libvirt and connman against 3.8-rc3 headers. Did this ever get resolved ? Libvirt has managed to work around the kernel issue in the meantime. But just today, I hit the same issue with the latest kernel-headers-3.8.2-206.fc18.x86_64 on Fedora 18 when backporting to an older libvirt branch that did not have the workaround. A kernel person would have to speak up for certain, but I think it is still a problem. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [PATCH v13 5/8] add a new qevent: QEVENT_GUEST_PANICKED
On 02/28/2013 05:13 AM, Hu Tao wrote: > This event will be emited when the guest is panicked. > > Signed-off-by: Wen Congyang > --- > include/monitor/monitor.h | 1 + > monitor.c | 1 + > 2 files changed, 2 insertions(+) Missing documentation in QMP/qmp-events.txt -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [PATCH v13 1/8] save/load cpu runstate
On 03/01/2013 12:36 AM, Hu Tao wrote: > On Thu, Feb 28, 2013 at 02:12:37PM -0700, Eric Blake wrote: >> On 02/28/2013 05:13 AM, Hu Tao wrote: >>> This patch enables preservation of cpu runstate during save/load vm. >>> So when a vm is restored from snapshot, the cpu runstate is restored, >>> too. >> >> What happens if a management app wants to override the runstate when >> restoring the domain? I can think of several useful scenarios: >> >> 1. management app pauses the guest, then saves domain state and other >> things (management state, or disk clones), then resumes the guest. >> Later, the management wants to revert to the saved state, but have the >> guest running right away. I guess here, knowing that the guest was >> saved in a paused state doesn't hurt, since the management app can >> resume it right away. >> >> 2. management app saves domain state of a live guest, then copies that >> state elsewhere. In its new location, the management app wants to >> investigate the state for forensic analysis - so even though the guest >> remembers that it was running, management wants to start it paused. >> Here, it is important that there must not be a window of time where the >> guest can run, otherwise, the results are not reproducible. > > -S takes precedence in the case. But for in-migration, runstate is > loaded from src. Given your answer, I think we're okay from the libvirt perspective. My biggest worry about a window where the guest runs unchecked is not a problem, given that libvirt always uses -S on incoming migration. In turn, libvirt has its own mechanisms for tracking whether the outgoing migration was started from a running state, along with API overrides to let a user override whether libvirt will resume the guest on the incoming migration side. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [PATCH v13 1/8] save/load cpu runstate
On 03/01/2013 12:36 AM, Hu Tao wrote: On Thu, Feb 28, 2013 at 02:12:37PM -0700, Eric Blake wrote: On 02/28/2013 05:13 AM, Hu Tao wrote: This patch enables preservation of cpu runstate during save/load vm. So when a vm is restored from snapshot, the cpu runstate is restored, too. What happens if a management app wants to override the runstate when restoring the domain? I can think of several useful scenarios: 1. management app pauses the guest, then saves domain state and other things (management state, or disk clones), then resumes the guest. Later, the management wants to revert to the saved state, but have the guest running right away. I guess here, knowing that the guest was saved in a paused state doesn't hurt, since the management app can resume it right away. 2. management app saves domain state of a live guest, then copies that state elsewhere. In its new location, the management app wants to investigate the state for forensic analysis - so even though the guest remembers that it was running, management wants to start it paused. Here, it is important that there must not be a window of time where the guest can run, otherwise, the results are not reproducible. -S takes precedence in the case. But for in-migration, runstate is loaded from src. Given your answer, I think we're okay from the libvirt perspective. My biggest worry about a window where the guest runs unchecked is not a problem, given that libvirt always uses -S on incoming migration. In turn, libvirt has its own mechanisms for tracking whether the outgoing migration was started from a running state, along with API overrides to let a user override whether libvirt will resume the guest on the incoming migration side. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [PATCH v13 5/8] add a new qevent: QEVENT_GUEST_PANICKED
On 02/28/2013 05:13 AM, Hu Tao wrote: This event will be emited when the guest is panicked. Signed-off-by: Wen Congyang we...@cn.fujitsu.com --- include/monitor/monitor.h | 1 + monitor.c | 1 + 2 files changed, 2 insertions(+) Missing documentation in QMP/qmp-events.txt -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [PATCH v13 1/8] save/load cpu runstate
On 02/28/2013 05:13 AM, Hu Tao wrote: > This patch enables preservation of cpu runstate during save/load vm. > So when a vm is restored from snapshot, the cpu runstate is restored, > too. What happens if a management app wants to override the runstate when restoring the domain? I can think of several useful scenarios: 1. management app pauses the guest, then saves domain state and other things (management state, or disk clones), then resumes the guest. Later, the management wants to revert to the saved state, but have the guest running right away. I guess here, knowing that the guest was saved in a paused state doesn't hurt, since the management app can resume it right away. 2. management app saves domain state of a live guest, then copies that state elsewhere. In its new location, the management app wants to investigate the state for forensic analysis - so even though the guest remembers that it was running, management wants to start it paused. Here, it is important that there must not be a window of time where the guest can run, otherwise, the results are not reproducible. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [PATCH v13 1/8] save/load cpu runstate
On 02/28/2013 05:13 AM, Hu Tao wrote: This patch enables preservation of cpu runstate during save/load vm. So when a vm is restored from snapshot, the cpu runstate is restored, too. What happens if a management app wants to override the runstate when restoring the domain? I can think of several useful scenarios: 1. management app pauses the guest, then saves domain state and other things (management state, or disk clones), then resumes the guest. Later, the management wants to revert to the saved state, but have the guest running right away. I guess here, knowing that the guest was saved in a paused state doesn't hurt, since the management app can resume it right away. 2. management app saves domain state of a live guest, then copies that state elsewhere. In its new location, the management app wants to investigate the state for forensic analysis - so even though the guest remembers that it was running, management wants to start it paused. Here, it is important that there must not be a window of time where the guest can run, otherwise, the results are not reproducible. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [libvirt] if_bridge.h: include in6.h for struct in6_addr use
On 01/13/2013 01:05 PM, Thomas Backlund wrote: > Thomas Backlund skrev 13.1.2013 20:38: >> patch both inline and attached as thunderbird tends to mess up ... > >> - >> >> if_bridge.h uses struct in6_addr ip6; but does not include the in6.h >> header. >> >> Found by trying to build libvirt and connman against 3.8-rc3 headers. >> > > Ok, > ignore this patch, it's not the correct fix as it introduces > redefinitions... > > Btw, the error that I hit that made me suggest this fix was libvirt > config check bailing out: > > config.log:/usr/include/linux/if_bridge.h:173:20: error: field 'ip6' has > incomplete type Hmm, just now noticing this thread, after independently hitting and and having to work around the same problem in libvirt: https://www.redhat.com/archives/libvir-list/2013-January/msg00930.html https://bugzilla.redhat.com/show_bug.cgi?id=895141 >> --- linux-3.8-rc3/include/uapi/linux/if_bridge.h2013-01-13 >> 20:09:54.257271755 +0200 >> +++ linux-3.8-rc3.fix/include/uapi/linux/if_bridge.h2013-01-13 >> 20:15:04.153676151 +0200 >> @@ -14,6 +14,7 @@ >> #define _UAPI_LINUX_IF_BRIDGE_H >> >> #include >> +#include >> -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [libvirt] if_bridge.h: include in6.h for struct in6_addr use
On 01/13/2013 01:05 PM, Thomas Backlund wrote: Thomas Backlund skrev 13.1.2013 20:38: patch both inline and attached as thunderbird tends to mess up ... - if_bridge.h uses struct in6_addr ip6; but does not include the in6.h header. Found by trying to build libvirt and connman against 3.8-rc3 headers. Ok, ignore this patch, it's not the correct fix as it introduces redefinitions... Btw, the error that I hit that made me suggest this fix was libvirt config check bailing out: config.log:/usr/include/linux/if_bridge.h:173:20: error: field 'ip6' has incomplete type Hmm, just now noticing this thread, after independently hitting and and having to work around the same problem in libvirt: https://www.redhat.com/archives/libvir-list/2013-January/msg00930.html https://bugzilla.redhat.com/show_bug.cgi?id=895141 --- linux-3.8-rc3/include/uapi/linux/if_bridge.h2013-01-13 20:09:54.257271755 +0200 +++ linux-3.8-rc3.fix/include/uapi/linux/if_bridge.h2013-01-13 20:15:04.153676151 +0200 @@ -14,6 +14,7 @@ #define _UAPI_LINUX_IF_BRIDGE_H #include linux/types.h +#include linux/in6.h -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v8] kvm: notify host when the guest is panicked
On 08/13/2012 12:21 PM, Marcelo Tosatti wrote: > On Wed, Aug 08, 2012 at 10:43:01AM +0800, Wen Congyang wrote: >> We can know the guest is panicked when the guest runs on xen. >> But we do not have such feature on kvm. >> >> Another purpose of this feature is: management app(for example: >> libvirt) can do auto dump when the guest is panicked. If management >> app does not do auto dump, the guest's user can do dump by hand if >> he sees the guest is panicked. >> >> We have three solutions to implement this feature: >> 1. use vmcall >> 2. use I/O port >> 3. use virtio-serial. >> >> We have decided to avoid touching hypervisor. The reason why I choose >> choose the I/O port is: >> 1. it is easier to implememt >> 2. it does not depend any virtual device >> 3. it can work when starting the kernel > > How about searching for the "Kernel panic - not syncing" string > in the guests serial output? Say libvirtd could take an action upon > that? > > Advantages: > - It works for all architectures. > - It does not depend on any virtual device. But it _does_ depend on a serial console, and furthermore requires libvirt to tee the serial console (right now, libvirt can treat the console as an opaque pass-through to the end user, but if you expect libvirt to parse the serial console for a particular string, you've lost some efficiency). > - It works as early as serial console output does (panics before > that should be rare). > - It allows you to see why the guest panicked. I think your arguments for a serial console have already been made and refuted in earlier versions of this patch series, which is WHY this series is still applicable. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v8] kvm: notify host when the guest is panicked
On 08/13/2012 12:21 PM, Marcelo Tosatti wrote: On Wed, Aug 08, 2012 at 10:43:01AM +0800, Wen Congyang wrote: We can know the guest is panicked when the guest runs on xen. But we do not have such feature on kvm. Another purpose of this feature is: management app(for example: libvirt) can do auto dump when the guest is panicked. If management app does not do auto dump, the guest's user can do dump by hand if he sees the guest is panicked. We have three solutions to implement this feature: 1. use vmcall 2. use I/O port 3. use virtio-serial. We have decided to avoid touching hypervisor. The reason why I choose choose the I/O port is: 1. it is easier to implememt 2. it does not depend any virtual device 3. it can work when starting the kernel How about searching for the Kernel panic - not syncing string in the guests serial output? Say libvirtd could take an action upon that? Advantages: - It works for all architectures. - It does not depend on any virtual device. But it _does_ depend on a serial console, and furthermore requires libvirt to tee the serial console (right now, libvirt can treat the console as an opaque pass-through to the end user, but if you expect libvirt to parse the serial console for a particular string, you've lost some efficiency). - It works as early as serial console output does (panics before that should be rare). - It allows you to see why the guest panicked. I think your arguments for a serial console have already been made and refuted in earlier versions of this patch series, which is WHY this series is still applicable. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
