[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 05/42] ivshmem-server: Don't overload POSIX s
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v2 05/42] ivshmem-server: Don't overload POSIX shmem and file name |
Date: |
Wed, 09 Mar 2016 21:14:16 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Marc-André Lureau <address@hidden> writes:
> Hi
>
> On Mon, Mar 7, 2016 at 8:25 PM, Markus Armbruster <address@hidden> wrote:
>> Option -m NAME is interpreted as directory name if we can statfs() it
>> and its on hugetlbfs. Else it's interpreted as POSIX shared memory
>> object name. This is nuts.
>>
>> Always interpret -m as directory. Create new -M for POSIX shared
>> memory. Last of -m or -M wins.
>>
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>
> I don't see why the last should win is a good idea, see attached patch
Last one wins is pretty common behavior. In fact, it's what this
program does for every single option with an argument. I didn't feel
like making -m and -M special.
> for a possible solution, also changing a few comments. Feel free to
> squash it in this patch or include it in your series.
I got a few comments inline.
> Reviewed-by: Marc-André Lureau <address@hidden>
Thanks!
[...]
> From e8112678496fd873ceaa34b3169e516130075ed4 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= <address@hidden>
> Date: Tue, 8 Mar 2016 20:31:09 +0100
> Subject: [PATCH] ivshmem-server: expect either -m or -M
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
> contrib/ivshmem-server/main.c | 21 ++++++++++++---------
> 1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/contrib/ivshmem-server/main.c b/contrib/ivshmem-server/main.c
> index 2795db5..368fc67 100644
> --- a/contrib/ivshmem-server/main.c
> +++ b/contrib/ivshmem-server/main.c
> @@ -77,7 +77,7 @@ ivshmem_server_parse_args(IvshmemServerArgs *args, int
> argc, char *argv[])
> "F" /* foreground */
> "p:" /* pid_file */
> "S:" /* unix_socket_path */
> - "m:" /* shm_path */
> + "m:" /* dirname */
The existing comments all name the member of args set by the option.
There is no member dirname.
> "M:" /* shm_path */
> "l:" /* shm_size */
> "n:" /* n_vectors */
> @@ -106,13 +106,15 @@ ivshmem_server_parse_args(IvshmemServerArgs *args, int
> argc, char *argv[])
> break;
>
> case 'M': /* shm_path */
> - args->shm_path = optarg;
> - args->use_shm_open = true;
> - break;
> + case 'm': /* dirname */
> + if (args->shm_path) {
> + fprintf(stderr, "Please specify either -m or -M.\n");
> + ivshmem_server_help(argv[0]);
> + exit(1);
> + }
>
> - case 'm': /* shm_path */
> args->shm_path = optarg;
> - args->use_shm_open = false;
> + args->use_shm_open = c == 'M';
I think I'll steal this idea :)
> break;
>
> case 'l': /* shm_size */
> @@ -207,7 +209,7 @@ main(int argc, char *argv[])
> .foreground = IVSHMEM_SERVER_DEFAULT_FOREGROUND,
> .pid_file = IVSHMEM_SERVER_DEFAULT_PID_FILE,
> .unix_socket_path = IVSHMEM_SERVER_DEFAULT_UNIX_SOCK_PATH,
> - .shm_path = IVSHMEM_SERVER_DEFAULT_SHM_PATH,
> + .shm_path = NULL,
> .use_shm_open = true,
> .shm_size = IVSHMEM_SERVER_DEFAULT_SHM_SIZE,
> .n_vectors = IVSHMEM_SERVER_DEFAULT_N_VECTORS,
> @@ -237,8 +239,9 @@ main(int argc, char *argv[])
>
> /* init the ivshms structure */
> if (ivshmem_server_init(&server, args.unix_socket_path,
> - args.shm_path, args.use_shm_open,
> - args.shm_size, args.n_vectors, args.verbose) <
> 0) {
> + args.shm_path ?: IVSHMEM_SERVER_DEFAULT_SHM_PATH,
> + args.use_shm_open, args.shm_size, args.n_vectors,
> + args.verbose) < 0) {
> fprintf(stderr, "cannot init server\n");
> goto err;
> }
- Re: [Qemu-devel] [PATCH v2 21/42] ivshmem: Clean up MSI-X conditions, (continued)
- [Qemu-devel] [PATCH v2 03/42] target-ppc: Document TOCTTOU in hugepage support, Markus Armbruster, 2016/03/07
- [Qemu-devel] [PATCH v2 06/42] qemu-doc: Fix ivshmem huge page example, Markus Armbruster, 2016/03/07
- [Qemu-devel] [PATCH v2 35/42] ivshmem: Inline check_shm_size() into its only caller, Markus Armbruster, 2016/03/07
- [Qemu-devel] [PATCH v2 20/42] ivshmem: Clean up register callbacks, Markus Armbruster, 2016/03/07
- [Qemu-devel] [PATCH v2 05/42] ivshmem-server: Don't overload POSIX shmem and file name, Markus Armbruster, 2016/03/07
[Qemu-devel] [PATCH v2 16/42] ivshmem: Drop ivshmem_event() stub, Markus Armbruster, 2016/03/07
[Qemu-devel] [PATCH v2 23/42] ivshmem: Assert interrupts are set up once, Markus Armbruster, 2016/03/07
[Qemu-devel] [PATCH v2 24/42] ivshmem: Simplify rejection of invalid peer ID from server, Markus Armbruster, 2016/03/07
[Qemu-devel] [PATCH v2 36/42] qdev: New DEFINE_PROP_ON_OFF_AUTO, Markus Armbruster, 2016/03/07
[Qemu-devel] [PATCH v2 25/42] ivshmem: Disentangle ivshmem_read(), Markus Armbruster, 2016/03/07
[Qemu-devel] [PATCH v2 40/42] ivshmem: Drop ivshmem property x-memdev, Markus Armbruster, 2016/03/07