[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PATCH v2 02/13] block/iscsi:Remove redundant statement in iscsi_ope
From: |
Chenqun (kuhn) |
Subject: |
RE: [PATCH v2 02/13] block/iscsi:Remove redundant statement in iscsi_open() |
Date: |
Sat, 29 Feb 2020 02:21:02 +0000 |
>-----Original Message-----
>From: Kevin Wolf [mailto:address@hidden]
>Sent: Friday, February 28, 2020 6:55 PM
>To: Chenqun (kuhn) <address@hidden>
>Cc: address@hidden; address@hidden;
>address@hidden; Zhanghailiang <address@hidden>;
>Euler Robot <address@hidden>; Ronnie Sahlberg
><address@hidden>; Paolo Bonzini <address@hidden>; Peter
>Lieven <address@hidden>; Max Reitz <address@hidden>
>Subject: Re: [PATCH v2 02/13] block/iscsi:Remove redundant statement in
>iscsi_open()
>
>Am 28.02.2020 um 08:30 hat Chenqun (kuhn) geschrieben:
>> >-----Original Message-----
>> >From: Kevin Wolf [mailto:address@hidden]
>> >Sent: Thursday, February 27, 2020 6:31 PM
>> >To: Chenqun (kuhn) <address@hidden>
>> >Cc: address@hidden; address@hidden;
>> >address@hidden; Zhanghailiang
>> ><address@hidden>; Euler Robot
>> ><address@hidden>; Ronnie Sahlberg
><address@hidden>;
>> >Paolo Bonzini <address@hidden>; Peter Lieven <address@hidden>; Max
>> >Reitz <address@hidden>
>> >Subject: Re: [PATCH v2 02/13] block/iscsi:Remove redundant statement
>> >in
>> >iscsi_open()
>> >
>> >Am 27.02.2020 um 02:49 hat Chenqun (kuhn) geschrieben:
>> >> >-----Original Message-----
>> >> >From: Kevin Wolf [mailto:address@hidden]
>> >> >Sent: Wednesday, February 26, 2020 5:55 PM
>> >> >To: Chenqun (kuhn) <address@hidden>
>> >> >Cc: address@hidden; address@hidden;
>> >> >address@hidden; Zhanghailiang
>> >> ><address@hidden>; Euler Robot
>> >> ><address@hidden>; Ronnie Sahlberg
>> ><address@hidden>;
>> >> >Paolo Bonzini <address@hidden>; Peter Lieven <address@hidden>;
>> >> >Max Reitz <address@hidden>
>> >> >Subject: Re: [PATCH v2 02/13] block/iscsi:Remove redundant
>> >> >statement in
>> >> >iscsi_open()
>> >> >
>> >> >Am 26.02.2020 um 09:46 hat address@hidden geschrieben:
>> >> >> From: Chen Qun <address@hidden>
>> >> >>
>> >> >> Clang static code analyzer show warning:
>> >> >> block/iscsi.c:1920:9: warning: Value stored to 'flags' is never read
>> >> >> flags &= ~BDRV_O_RDWR;
>> >> >> ^ ~~~~~~~~~~~~
>> >> >>
>> >> >> Reported-by: Euler Robot <address@hidden>
>> >> >> Signed-off-by: Chen Qun <address@hidden>
>> >> >
>> >> >Hmm, I'm not so sure about this one because if we remove the line,
>> >> >flags will be inconsistent with bs->open_flags. It feels like
>> >> >setting a trap for anyone who wants to add code using flags in the
>future.
>> >> Hi Kevin,
>> >> I find it exists since 8f3bf50d34037266. : )
>> >
>> >Yes, it has existed from the start with auto-read-only.
>> >
>> >> It's not a big deal, just upset clang static code analyzer.
>> >> As you said, it could be a trap for the future.
>> >
>> >What's interesting is that we do have one user of the flags later in
>> >the function, but it uses bs->open_flags instead:
>> >
>> > ret = iscsi_allocmap_init(iscsilun, bs->open_flags);
>> >
>> >Maybe this should be using flags? (The value of the bits we're
>> >interested in is the same, but when flags is passed as a parameter, I
>> >would expect it to be
>> >used.)
>> >
>> Hi Kevin,
>> I have a question: are 'flags' exactly the same as 'bs-> open_flags'?
>> In the function bdrv_open_common() at block.c file, the existence of
>statement( open_flags = bdrv_open_flags(bs, bs->open_flags); ) makes them
>a little different.
>> Will this place affect them inconsistently ?
>
>Not exactly the same, that's why I said "value of the bits we're interested in
>is
>the same". bdrv_open_flags() basically just filters out things that are handled
>by the generic block layer and none of the block driver's business.
>
>To be precise, iscsi_allocmap_init() only checks BDRV_O_NOCACHE, which is
>the same in both.
>
>> Is it safer if we assign bs-> open_flags to flags?
>
>This would add back the flags that we consciously filtered out before, so I
>would say no.
>
Well, I see, thank you very much for your detailed explanation!
Thanks.
- Re: [PATCH v2 03/13] block/file-posix: Remove redundant statement in raw_handle_perm_lock(), (continued)
- [PATCH v2 10/13] migration/vmstate: Remove redundant statement in vmstate_save_state_v(), kuhn.chenqun, 2020/02/26
- [PATCH v2 02/13] block/iscsi:Remove redundant statement in iscsi_open(), kuhn.chenqun, 2020/02/26
- Re: [PATCH v2 02/13] block/iscsi:Remove redundant statement in iscsi_open(), Kevin Wolf, 2020/02/26
- RE: [PATCH v2 02/13] block/iscsi:Remove redundant statement in iscsi_open(), Chenqun (kuhn), 2020/02/26
- Re: [PATCH v2 02/13] block/iscsi:Remove redundant statement in iscsi_open(), Kevin Wolf, 2020/02/27
- RE: [PATCH v2 02/13] block/iscsi:Remove redundant statement in iscsi_open(), Chenqun (kuhn), 2020/02/27
- RE: [PATCH v2 02/13] block/iscsi:Remove redundant statement in iscsi_open(), Chenqun (kuhn), 2020/02/28
- Re: [PATCH v2 02/13] block/iscsi:Remove redundant statement in iscsi_open(), Kevin Wolf, 2020/02/28
- RE: [PATCH v2 02/13] block/iscsi:Remove redundant statement in iscsi_open(),
Chenqun (kuhn) <=
[PATCH v2 08/13] display/blizzard: Remove redundant statement in blizzard_draw_line16_32(), kuhn.chenqun, 2020/02/26
[PATCH v2 09/13] dma/xlnx-zdma: Remove redundant statement in zdma_write_dst(), kuhn.chenqun, 2020/02/26
[PATCH v2 05/13] scsi/scsi-disk: Remove redundant statement in scsi_disk_emulate_command(), kuhn.chenqun, 2020/02/26
[PATCH v2 04/13] scsi/esp-pci: Remove redundant statement in esp_pci_io_write(), kuhn.chenqun, 2020/02/26
[PATCH v2 06/13] display/pxa2xx_lcd: Remove redundant statement in pxa2xx_palette_parse(), kuhn.chenqun, 2020/02/26
[PATCH v2 13/13] monitor/hmp-cmds: Remove redundant statement in hmp_rocker_of_dpa_groups(), kuhn.chenqun, 2020/02/26