[dm-devel] [PATCH v2] dm raid1: "mirror" target doesn't use all available legs on multiple failures
In case legs of a mirror target fail, any read will cause a new, operational default leg to be selected and the read be resubmitted to it. If that new default leg fails the read too, no other still accessible legs are used to resubmit the read again thus failing the io. Fix by allowing the read to get resubmitted until there's no operational legs any more. Removes any details.bi_dev use as a flag. Resolves: rhbz1383444 Signed-off-by: Heinz Mauelshagen --- drivers/md/dm-raid1.c | 16 1 file changed, 16 deletions(-) diff --git a/drivers/md/dm-raid1.c b/drivers/md/dm-raid1.c index 7a6254d..a7fe697 100644 --- a/drivers/md/dm-raid1.c +++ b/drivers/md/dm-raid1.c @@ -145,7 +145,6 @@ static void dispatch_bios(void *context, struct bio_list *bio_list) struct dm_raid1_bio_record { struct mirror *m; - /* if details->bi_bdev == NULL, details were not saved */ struct dm_bio_details details; region_t write_region; }; @@ -1200,8 +1199,6 @@ static int mirror_map(struct dm_target *ti, struct bio *bio) struct dm_raid1_bio_record *bio_record = dm_per_bio_data(bio, sizeof(struct dm_raid1_bio_record)); - bio_record->details.bi_bdev = NULL; - if (rw == WRITE) { /* Save region for mirror_end_io() handler */ bio_record->write_region = dm_rh_bio_to_region(ms->rh, bio); @@ -1266,16 +1263,6 @@ static int mirror_end_io(struct dm_target *ti, struct bio *bio, int error) goto out; if (unlikely(error)) { - if (!bio_record->details.bi_bdev) { - /* -* There wasn't enough memory to record necessary -* information for a retry or there was no other -* mirror in-sync. -*/ - DMERR_LIMIT("Mirror read failed."); - return -EIO; - } - m = bio_record->m; DMERR("Mirror read failed from %s. Trying alternative device.", @@ -1291,7 +1278,6 @@ static int mirror_end_io(struct dm_target *ti, struct bio *bio, int error) bd = &bio_record->details; dm_bio_restore(bd, bio); - bio_record->details.bi_bdev = NULL; bio->bi_error = 0; queue_bio(ms, bio, rw); @@ -1301,8 +1287,6 @@ static int mirror_end_io(struct dm_target *ti, struct bio *bio, int error) } out: - bio_record->details.bi_bdev = NULL; - return error; } -- 2.7.4 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH] multipath-tools: remove hwhandler when it's alua
On Wed, Oct 12, 2016 at 01:06:10PM +0200, Xose Vazquez Perez wrote: > On 10/11/2016 08:22 AM, Hannes Reinecke wrote: > > > Autodetection will only work if the hardware handler is loaded. > > If the handler is _not_ loaded autodetection won't work, either. > > And if we add this patch multipath will never load the modules, neither. > > So we need this functionality as a fallback if autodetect does not work. > > I'm pretty sure all works flawlessly with this patch. Because I use similar > options for EMC VNX7500(CLARiiON) configured as ALUA "Failover Mode 4 > (Asymmetric Active/Active)", > with SLES-11/12 and RHEL-6 booting from SAN. > > For SLES it was added to DGC config: > retain_attached_hw_handler yes > detect_prio yes > > In RHEL it's included by default: > http://pkgs.fedoraproject.org/cgit/rpms/device-mapper-multipath.git/plain/0117-RHBZ-1198424-autodetect-clariion-alua.patch > > And the modules are preloaded by multipathd/multipathd.service: > ExecStartPre=/sbin/modprobe -a scsi_dh_alua scsi_dh_emc scsi_dh_rdac > dm-multipath > > > RDAC devices also depend *exclusively* on "retain_attached_hw_handler" and > "detect_prio" > to run in ALUA mode. The purpose of the the "retain_attached_hw_handler" and "detect_prio" options were to allow devices that support ALUA and non-ALUA modes, to detect which mode the device was in, so that the builtin configuration would work correctly for both modes. If a device is ALUA only, I don't see any benefit to not hardcoding that. Simply from a ease-of-use persepective, having ALUA in the configuration make it obvious how the device is supposed to be configured. But more importantly, as has already been metioned, it's more robust to hardcode the ALUA handler for the devices that need it in case the handler was not attached before multipath started working on the device. Also, if I recall correctly, for the device handler to get attached correctly, we don't just need the device handler module isntalled before multipathd starts. We need it installed when the path device is initially discovered. The more I think about this, the more I think we might want to revisit some to the configuration simplifications that have been made. In some cases, I think they went too far. For instance, most devices will work correctly regardless of the values for things like "rr_minio_rq" and "path_selector", so I have no objection to these values being removed from the device configuartion, if they are the same as the default values. On the other hand, we have things like "hardware_handler" and "path_checker", where the device simply will not function correctly with certain values. For these attributes, it makes sense to leave them in the device configuration, even if they are the same as the default values. The goal should be that you can't break any device configurations by changing the default values. -Ben -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH] multipath-tools: add Xiotech ISE arrays to hwtable
Based on documentation provided by the manufacturer: https://drive.google.com/file/d/0B_B6YmEmO7cDc201a3ZlcmFvUmM Cc: Christophe Varoqui Cc: device-mapper development Signed-off-by: Xose Vazquez Perez --- libmultipath/hwtable.c | 9 + 1 file changed, 9 insertions(+) diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c index 4fffeb7..dc3010a 100644 --- a/libmultipath/hwtable.c +++ b/libmultipath/hwtable.c @@ -933,6 +933,15 @@ static struct hwentry default_hw[] = { .prio_name = PRIO_ALUA, .no_path_retry = 15, }, + /* +* Xiotech +*/ + { + .vendor= "(XIOTECH|XIOtech)", + .product = "ISE", + .pgpolicy = MULTIBUS, + .no_path_retry = 12, + }, #if 0 /* * Copy this TEMPLATE to add new hardware. -- 2.10.1 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH] multipath-tools: remove hwhandler when it's alua
On 10/11/2016 08:43 AM, Christophe Varoqui wrote: > so Xosé, can you confirm the last introduced hwtable entries don't need the > alua hwhandler : Yes, defined: > - tegile > - nimble No needed/defined: > - nexsan > - nfinidat - NexGen|Pivot3 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH] multipathd: fix issue in 'map $map getprstatus' reply
On 10/11/16 20:03, peng.lia...@zte.com.cn wrote: > From: peng liang > > -add missing newline to 'map|multipath $map getprstatus' reply > -use asprintf instead of sprintf > > Signed-off-by: peng liang > --- > multipathd/cli_handlers.c | 14 ++ > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c > index 8ff4362..16445ea 100644 > --- a/multipathd/cli_handlers.c > +++ b/multipathd/cli_handlers.c > @@ -1,6 +1,9 @@ > /* > * Copyright (c) 2005 Christophe Varoqui > */ > +#define _GNU_SOURCE > + > +#include > #include "checkers.h" > #include "memory.h" > #include "vector.h" > @@ -1285,14 +1288,9 @@ cli_getprstatus (void * v, char ** reply, int * > len, void * data) > > condlog(3, "%s: prflag = %u", param, (unsigned > int)mpp->prflag); > > - *reply =(char *)malloc(2); > - *len = 2; > - memset(*reply,0,2); > - > - > - sprintf(*reply,"%d",mpp->prflag); > - (*reply)[1]='\0'; > - > + *len = asprintf(reply, "%d\n", mpp->prflag); > + if (*len < 0) > + return 1; > > condlog(3, "%s: reply = %s", param, *reply); Hello Peng, Sorry but returning 1 looks somewhat inconsistent to me. This function is called indirectly by uxsock_trigger() and that function expects that cli_getprstatus() either returns a positive error code (E...) or a negative error code. Please change this patch such that ENOMEM is returned instead of 1 if asprintf() fails. Thanks, Bart. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH] multipath-tools: add IBM/FlashSystem to hwtable
On 10/07/2016 04:24 PM, Steffen Maier wrote: > You'll notice that we from Linux on s390x have a bit different recommendations > for some of the values (no_path_retry actually depends on whether you have a > cluster software/filesystem that can cope with EIO on last-path-loss. > Currently, > I see the default without that, i.e. non-clustered from which we would like > to hide any path issues). This has to be modified manually. > Is there some way to have "dependent" settings? No. multipath.conf is mainly static. Only two options are dynamically reconfigured, ALUA-prio and the hardware handler with "detect_prio" and "retain_attached_hw_handler". -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH] multipath-tools: preload also scsi_dh_hp_sw before run multipathd
On 10/12/2016 02:43 PM, Hannes Reinecke wrote: > On 10/12/2016 01:37 PM, Xose Vazquez Perez wrote: >> scsi_dh_hp_sw is used also for dec/compaq ESA12000 and RA8000 arrays with >> HSG80 controller. >> And in HP MSA 1000/1500 and EVA 3000/5000, with old firmware. >> > EVA? I sincerely doubt that. > Do you really have an example for that? { /* MSA 1000/1500 and EVA 3000/5000, with old firmware */ .vendor= "(COMPAQ|HP)", .product = "(MSA|HSV)1[01]0", .hwhandler = "1 hp_sw", .pgpolicy = GROUP_BY_PRIO, .no_path_retry = 12, .checker_name = HP_SW, .prio_name = PRIO_HP_SW, }, HSV100 is HP StorageWorks Enterprise Virtual Array 3000: http://h18000.www1.hp.com/products/quickspecs/archives_Division/11619_div_v9/11619_div.PDF HSV110 is HP StorageWorks Enterprise Virtual Array 5000: https://www.hpe.com/h20195/v2/GetPDF.aspx/c04282828.pdf -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH] multipath-tools: preload also scsi_dh_hp_sw before run multipathd
On 10/12/2016 01:37 PM, Xose Vazquez Perez wrote: On 10/11/2016 03:41 PM, Hannes Reinecke wrote: On 10/11/2016 02:55 PM, Xose Vazquez Perez wrote: Cc: Hannes Reinecke Cc: Christophe Varoqui Cc: device-mapper development Signed-off-by: Xose Vazquez Perez --- multipathd/multipathd.service | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/multipathd/multipathd.service b/multipathd/multipathd.service index e3d6f91..45aca35 100644 --- a/multipathd/multipathd.service +++ b/multipathd/multipathd.service @@ -11,7 +11,7 @@ Conflicts=shutdown.target Type=notify NotifyAccess=main LimitCORE=infinity -ExecStartPre=/sbin/modprobe -a scsi_dh_alua scsi_dh_emc scsi_dh_rdac dm-multipath +ExecStartPre=/sbin/modprobe -a scsi_dh_alua scsi_dh_emc scsi_dh_hp_sw scsi_dh_rdac dm-multipath ExecStart=/sbin/multipathd -d -s ExecReload=/sbin/multipathd reconfigure If you can point me to a single user of an MSA1500 I'm happy to include it. If you don't know what an MSA1500 is nor what hp_sw does it's probably not a good idea to have it. Why? scsi_dh_hp_sw is used also for dec/compaq ESA12000 and RA8000 arrays with HSG80 controller. And in HP MSA 1000/1500 and EVA 3000/5000, with old firmware. EVA? I sincerely doubt that. Do you really have an example for that? Anyway. Point here is, these are _really_ old SCSI parallel arrays. And the failover command is _really_ stupid (just send a START STOP UNIT to the passive path). As this has a tendency to interfere with normal operation I prefer to _NOT_ have it loaded per default; in fact, I'm on the verge of removing it altogether. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH] multipath-tools: preload also scsi_dh_hp_sw before run multipathd
On 10/11/2016 03:41 PM, Hannes Reinecke wrote: > On 10/11/2016 02:55 PM, Xose Vazquez Perez wrote: >> Cc: Hannes Reinecke >> Cc: Christophe Varoqui >> Cc: device-mapper development >> Signed-off-by: Xose Vazquez Perez >> --- >> multipathd/multipathd.service | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/multipathd/multipathd.service b/multipathd/multipathd.service >> index e3d6f91..45aca35 100644 >> --- a/multipathd/multipathd.service >> +++ b/multipathd/multipathd.service >> @@ -11,7 +11,7 @@ Conflicts=shutdown.target >> Type=notify >> NotifyAccess=main >> LimitCORE=infinity >> -ExecStartPre=/sbin/modprobe -a scsi_dh_alua scsi_dh_emc scsi_dh_rdac >> dm-multipath >> +ExecStartPre=/sbin/modprobe -a scsi_dh_alua scsi_dh_emc scsi_dh_hp_sw >> scsi_dh_rdac dm-multipath >> ExecStart=/sbin/multipathd -d -s >> ExecReload=/sbin/multipathd reconfigure >> > If you can point me to a single user of an MSA1500 I'm happy to include it. > If you don't > know what an MSA1500 is nor what hp_sw does it's probably not a good idea to > have it. Why? scsi_dh_hp_sw is used also for dec/compaq ESA12000 and RA8000 arrays with HSG80 controller. And in HP MSA 1000/1500 and EVA 3000/5000, with old firmware. Thank you. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH] multipath-tools: remove hwhandler when it's alua
On 10/11/2016 08:22 AM, Hannes Reinecke wrote: > Autodetection will only work if the hardware handler is loaded. > If the handler is _not_ loaded autodetection won't work, either. > And if we add this patch multipath will never load the modules, neither. > So we need this functionality as a fallback if autodetect does not work. I'm pretty sure all works flawlessly with this patch. Because I use similar options for EMC VNX7500(CLARiiON) configured as ALUA "Failover Mode 4 (Asymmetric Active/Active)", with SLES-11/12 and RHEL-6 booting from SAN. For SLES it was added to DGC config: retain_attached_hw_handler yes detect_prio yes In RHEL it's included by default: http://pkgs.fedoraproject.org/cgit/rpms/device-mapper-multipath.git/plain/0117-RHBZ-1198424-autodetect-clariion-alua.patch And the modules are preloaded by multipathd/multipathd.service: ExecStartPre=/sbin/modprobe -a scsi_dh_alua scsi_dh_emc scsi_dh_rdac dm-multipath RDAC devices also depend *exclusively* on "retain_attached_hw_handler" and "detect_prio" to run in ALUA mode. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH] multipath-tools: release lock on handler failure
Inside parse_cmd() the pthread_cleanup_pop() rely on '!r' as the indicator of locked or not, while this will be overwritten if the handler return failed, and the unlock will be missing. This will lead into the situation that all the following operation will trying to hold a lock which will never be released. This patch using a separate flag to record the status of locking to make sure the unlock and lock are in pairs. Signed-off-by: Michael Wang --- multipathd/cli.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/multipathd/cli.c b/multipathd/cli.c index e8a9384..50161be 100644 --- a/multipathd/cli.c +++ b/multipathd/cli.c @@ -481,6 +481,7 @@ parse_cmd (char * cmd, char ** reply, int * len, void * data, int timeout ) tmo.tv_sec = 0; } if (h->locked) { + int locked = 0; struct vectors * vecs = (struct vectors *)data; pthread_cleanup_push(cleanup_lock, &vecs->lock); @@ -491,10 +492,11 @@ parse_cmd (char * cmd, char ** reply, int * len, void * data, int timeout ) r = 0; } if (r == 0) { + locked = 1; pthread_testcancel(); r = h->fn(cmdvec, reply, len, data); } - pthread_cleanup_pop(!r); + pthread_cleanup_pop(locked); } else r = h->fn(cmdvec, reply, len, data); free_keys(cmdvec); -- 2.5.0 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] 答复: [RFC] unreleased lock after handler failure
Hi, Tang On 10/12/2016 10:33 AM, tang.jun...@zte.com.cn wrote: > Hi, Michael Wang > > This patch looks well for me, cleanup_lock() should be called without > affectedby the return value of h->fn(). Thanks for the quick respond, I'll submit a formal patch then :-) Regards, Michael Wang > > Regards, > Tang > > > > 发件人: Michael Wang > 收件人: christophe.varo...@opensvc.com, dm-devel@redhat.com, > 日期: 2016/10/12 16:11 > 主题:[dm-devel] [RFC] unreleased lock after handler failure > 发件人:dm-devel-boun...@redhat.com > -- > > > > Hi, folks > > While we are testing with the latest multipathd, we encounter issue that once > 'del map' failed, all the following operation will show 'error -1 receiving > packet' > whatever how long the timeout has been set (we have applied the latest fix > which > make sure the poll will last for 10 seconds). > > After some debugging we found that inside parse_cmd(), the > pthread_cleanup_pop() > rely on '!r' as indicator for locked or not, while this will be overwritten if > the handler return failed (1 in our case), and then the unlock will be missed. > > After applied below fix the problem got solved, although the del map failed, > the > other operation can still works. > > Could you please help to review whether this is really the problem to be fixed > like that? > > Regards, > Michael Wang > > > > diff --git a/multipathd/cli.c b/multipathd/cli.c > index e8a9384..50161be 100644 > --- a/multipathd/cli.c > +++ b/multipathd/cli.c > @@ -481,6 +481,7 @@ parse_cmd (char * cmd, char ** reply, int * len, void * > data, int timeout ) >tmo.tv_sec = 0; >} >if (h->locked) { > + int locked = 0; >struct vectors * vecs = (struct vectors *)data; > >pthread_cleanup_push(cleanup_lock, &vecs->lock); > @@ -491,10 +492,11 @@ parse_cmd (char * cmd, char ** reply, int * len, void * > data, int timeout ) >r = 0; >} >if (r == 0) { > + locked = 1; >pthread_testcancel(); >r = h->fn(cmdvec, reply, len, data); >} > - pthread_cleanup_pop(!r); > + pthread_cleanup_pop(locked); >} else >r = h->fn(cmdvec, reply, len, data); >free_keys(cmdvec); > > -- > dm-devel mailing list > dm-devel@redhat.com > https://www.redhat.com/mailman/listinfo/dm-devel > -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] 答复: [RFC] unreleased lock after handler failure
Hi, Michael Wang This patch looks well for me, cleanup_lock() should be called without affected by the return value of h->fn(). Regards, Tang 发件人: Michael Wang 收件人: christophe.varo...@opensvc.com, dm-devel@redhat.com, 日期: 2016/10/12 16:11 主题: [dm-devel] [RFC] unreleased lock after handler failure 发件人: dm-devel-boun...@redhat.com Hi, folks While we are testing with the latest multipathd, we encounter issue that once 'del map' failed, all the following operation will show 'error -1 receiving packet' whatever how long the timeout has been set (we have applied the latest fix which make sure the poll will last for 10 seconds). After some debugging we found that inside parse_cmd(), the pthread_cleanup_pop() rely on '!r' as indicator for locked or not, while this will be overwritten if the handler return failed (1 in our case), and then the unlock will be missed. After applied below fix the problem got solved, although the del map failed, the other operation can still works. Could you please help to review whether this is really the problem to be fixed like that? Regards, Michael Wang diff --git a/multipathd/cli.c b/multipathd/cli.c index e8a9384..50161be 100644 --- a/multipathd/cli.c +++ b/multipathd/cli.c @@ -481,6 +481,7 @@ parse_cmd (char * cmd, char ** reply, int * len, void * data, int timeout ) tmo.tv_sec = 0; } if (h->locked) { + int locked = 0; struct vectors * vecs = (struct vectors *)data; pthread_cleanup_push(cleanup_lock, &vecs->lock); @@ -491,10 +492,11 @@ parse_cmd (char * cmd, char ** reply, int * len, void * data, int timeout ) r = 0; } if (r == 0) { + locked = 1; pthread_testcancel(); r = h->fn(cmdvec, reply, len, data); } - pthread_cleanup_pop(!r); + pthread_cleanup_pop(locked); } else r = h->fn(cmdvec, reply, len, data); free_keys(cmdvec); -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [RFC] unreleased lock after handler failure
Hi, folks While we are testing with the latest multipathd, we encounter issue that once 'del map' failed, all the following operation will show 'error -1 receiving packet' whatever how long the timeout has been set (we have applied the latest fix which make sure the poll will last for 10 seconds). After some debugging we found that inside parse_cmd(), the pthread_cleanup_pop() rely on '!r' as indicator for locked or not, while this will be overwritten if the handler return failed (1 in our case), and then the unlock will be missed. After applied below fix the problem got solved, although the del map failed, the other operation can still works. Could you please help to review whether this is really the problem to be fixed like that? Regards, Michael Wang diff --git a/multipathd/cli.c b/multipathd/cli.c index e8a9384..50161be 100644 --- a/multipathd/cli.c +++ b/multipathd/cli.c @@ -481,6 +481,7 @@ parse_cmd (char * cmd, char ** reply, int * len, void * data, int timeout ) tmo.tv_sec = 0; } if (h->locked) { + int locked = 0; struct vectors * vecs = (struct vectors *)data; pthread_cleanup_push(cleanup_lock, &vecs->lock); @@ -491,10 +492,11 @@ parse_cmd (char * cmd, char ** reply, int * len, void * data, int timeout ) r = 0; } if (r == 0) { + locked = 1; pthread_testcancel(); r = h->fn(cmdvec, reply, len, data); } - pthread_cleanup_pop(!r); + pthread_cleanup_pop(locked); } else r = h->fn(cmdvec, reply, len, data); free_keys(cmdvec); -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 7/9] tests: Some additional checks for ioctl_dm test
--- tests/ioctl_dm.c | 505 ++ 1 file changed, 505 insertions(+) diff --git a/tests/ioctl_dm.c b/tests/ioctl_dm.c index 24232b7..0b2c5a7 100644 --- a/tests/ioctl_dm.c +++ b/tests/ioctl_dm.c @@ -2,13 +2,26 @@ #ifdef HAVE_LINUX_DM_IOCTL_H +# include # include +# include # include # include # include # include # include +# define STR32 "AbCdEfGhIjKlMnOpQrStUvWxYz012345" + +static const char str129[] = STR32 STR32 STR32 STR32 "6"; + +static const __u64 dts_sector_base = (__u64) 0xdeadca75facef157ULL; +static const __u64 dts_sector_step = (__u64) 0x10001ULL; +static const __u64 dts_length_base = (__u64) 0xbadc0dedda7a1057ULL; +static const __u64 dts_length_step = (__u64) 0x70007ULL; +static const __s32 dts_status_base = (__s32) 3141592653U; +static const __s32 dts_status_step = 0x1234; + static struct s { struct dm_ioctl ioc; union { @@ -24,6 +37,43 @@ static struct s { } u; } s; +struct dm_table_open_test { + struct dm_ioctl ioc; + struct dm_target_spec target0; + char param0[1]; + struct dm_target_spec target1; + char param1[2]; + struct dm_target_spec target2; + char param2[3]; + struct dm_target_spec target3; + char param3[4]; + struct dm_target_spec target4; + char param4[5]; + struct dm_target_spec target5; + char param5[6]; + struct dm_target_spec target6; + char param6[7]; + struct dm_target_spec target7; + char param7[8]; + struct dm_target_spec target8; + char param8[9]; + struct dm_target_spec target9; + char param9[10]; +}; + +struct dm_target_msg_test { + struct dm_ioctl ioc; + struct dm_target_msg msg; +}; + +struct args { + unsigned int arg; + const char *str; + bool has_params; + bool has_event_nr; +}; + + static void init_s(struct dm_ioctl *s, size_t size, size_t offs) { @@ -38,9 +88,147 @@ init_s(struct dm_ioctl *s, size_t size, size_t offs) strcpy(s->uuid, "uuu"); } +static void +init_dm_target_spec(struct dm_target_spec *ptr, uint32_t id) +{ + ptr->sector_start = dts_sector_base + dts_sector_step * id; + ptr->length = dts_length_base + dts_length_step * id; + ptr->status = dts_status_base + dts_status_step * id; + + strncpy(ptr->target_type, str129 + + id % (sizeof(str129) - sizeof(ptr->target_type)), + id % (sizeof(ptr->target_type) + 1)); + if (id % (sizeof(ptr->target_type) + 1) < sizeof(ptr->target_type)) + ptr->target_type[id % (sizeof(ptr->target_type) + 1)] = '\0'; +} + +static void +print_dm_target_spec(struct dm_target_spec *ptr, uint32_t id) +{ + printf("{sector_start=%" PRI__u64 ", length=%" PRI__u64 ", " + "target_type=\"%.*s\", string=", + dts_sector_base + dts_sector_step * id, + dts_length_base + dts_length_step * id, + (int) (id % (sizeof(ptr->target_type) + 1)), + str129 + id % (sizeof(str129) - sizeof(ptr->target_type))); +} + +# define ARG_STR(_arg) (_arg), #_arg + int main(void) { + /* We can't check these properly for now */ + static struct args dummy_check_cmds_nodev[] = { + { ARG_STR(DM_REMOVE_ALL),false }, + { ARG_STR(DM_LIST_DEVICES), true }, + { ARG_STR(DM_LIST_VERSIONS), true }, + }; + static struct args dummy_check_cmds[] = { + { ARG_STR(DM_DEV_CREATE),false }, + { ARG_STR(DM_DEV_REMOVE),false, true }, + { ARG_STR(DM_DEV_STATUS),false }, + { ARG_STR(DM_DEV_WAIT), true, true }, + { ARG_STR(DM_TABLE_CLEAR), false }, + { ARG_STR(DM_TABLE_DEPS),true }, + { ARG_STR(DM_TABLE_STATUS), true }, + }; + + struct dm_ioctl *dm_arg = + tail_alloc(sizeof(*dm_arg) - sizeof(dm_arg->data)); + struct dm_table_open_test *dm_arg_open1 = + tail_alloc(offsetof(struct dm_table_open_test, target1)); + struct dm_table_open_test *dm_arg_open2 = + tail_alloc(offsetof(struct dm_table_open_test, param1)); + struct dm_table_open_test *dm_arg_open3 = + tail_alloc(offsetof(struct dm_table_open_test, target9)); + struct dm_target_msg_test *dm_arg_msg = + tail_alloc(sizeof(*dm_arg_msg)); + + int saved_errno; + unsigned int i; + + + /* Incorrect operation */ + ioctl(-1, _IOW(DM_IOCTL, 0xde, int), dm_arg); + printf("ioctl(-1, _IOC(_IOC_WRITE, %#04x, 0xde, %#04zx), %p) = " + "-1 EBADF (%m)\n", + DM_IOCTL, sizeof(int), dm_arg); + + + /* DM_VERSION */ + /* Incorrect pointer */ + ioctl(-1, DM_VERSION, dm_arg + 1); + printf("ioctl(-1, DM_VERSION, %p) = -1 EBADF (%m)\n", dm_arg + 1); + +
[dm-devel] [PATCH 2/9] tests: Add check for printing of overlength strings to ioctl_dm test
--- tests/ioctl_dm.c|2 +- tests/ioctl_dm.test |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ioctl_dm.c b/tests/ioctl_dm.c index 6ad4ea9..0a3bbf4 100644 --- a/tests/ioctl_dm.c +++ b/tests/ioctl_dm.c @@ -77,7 +77,7 @@ main(void) printf("ioctl(-1, DM_DEV_SET_GEOMETRY, " "{version=4.1.2, data_size=%u, data_start=%u, " "dev=makedev(18, 52), name=\"nnn\", uuid=\"uuu\", flags=0, " - "string=\"10 20 30 40\"}) = -1 EBADF (%m)\n", + "string=\"10 20 30 \"...}) = -1 EBADF (%m)\n", s.ioc.data_size, s.ioc.data_start); init_s(&s.ioc, sizeof(s), offsetof(struct s, u)); diff --git a/tests/ioctl_dm.test b/tests/ioctl_dm.test index db77616..78866d3 100755 --- a/tests/ioctl_dm.test +++ b/tests/ioctl_dm.test @@ -5,7 +5,7 @@ . "${srcdir=.}/init.sh" run_prog > /dev/null -run_strace -a16 -veioctl $args > "$EXP" +run_strace -a16 -s9 -veioctl $args > "$EXP" check_prog grep grep -v '^ioctl([012],' < "$LOG" > "$OUT" match_diff "$OUT" "$EXP" -- 1.7.10.4 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 8/9] tests: Add ioctl_dm to .gitignore
--- tests/.gitignore |1 + 1 file changed, 1 insertion(+) diff --git a/tests/.gitignore b/tests/.gitignore index a6d014d..9045117 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -103,6 +103,7 @@ getxxid inet-cmsg ioctl ioctl_block +ioctl_dm ioctl_evdev ioctl_evdev-v ioctl_mtd -- 1.7.10.4 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 4/9] tests/ioctl_dm: whitespace
--- tests/ioctl_dm.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/ioctl_dm.c b/tests/ioctl_dm.c index 5f2689c..261983c 100644 --- a/tests/ioctl_dm.c +++ b/tests/ioctl_dm.c @@ -24,7 +24,8 @@ static struct s { } u; } s; -static void init_s(struct dm_ioctl *s, size_t size, size_t offs) +static void +init_s(struct dm_ioctl *s, size_t size, size_t offs) { memset(s, 0, size); s->version[0] = DM_VERSION_MAJOR; -- 1.7.10.4 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH] multipathd: fix issue in 'map $map getprstatus' reply
Please have a review for this patch, hope for your comments. Thanks 发件人: peng.lia...@zte.com.cn 收件人: christophe varoqui , 抄送: zhang.ka...@zte.com.cn, dm-devel@redhat.com, peng liang 日期: 2016-08-04 15:31 主题: [dm-devel] [PATCH] multipathd: fix issue in 'map $map getprstatus' reply 发件人: dm-devel-boun...@redhat.com From: peng liang -add missing newline to 'map|multipath $map getprstatus' reply -use asprintf instead of sprintf Signed-off-by: peng liang --- multipathd/cli_handlers.c | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c index 8ff4362..16445ea 100644 --- a/multipathd/cli_handlers.c +++ b/multipathd/cli_handlers.c @@ -1,6 +1,9 @@ /* * Copyright (c) 2005 Christophe Varoqui */ +#define _GNU_SOURCE + +#include #include "checkers.h" #include "memory.h" #include "vector.h" @@ -1285,14 +1288,9 @@ cli_getprstatus (void * v, char ** reply, int * len, void * data) condlog(3, "%s: prflag = %u", param, (unsigned int)mpp->prflag); -*reply =(char *)malloc(2); -*len = 2; -memset(*reply,0,2); - - -sprintf(*reply,"%d",mpp->prflag); -(*reply)[1]='\0'; - +*len = asprintf(reply, "%d\n", mpp->prflag); +if (*len < 0) +return 1; condlog(3, "%s: reply = %s", param, *reply); -- 2.8.1.windows.1 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 0/9] Additional checks for strace DM ioctl decoder test
Hello. Aside from additional checks themselves, this patchset also contains two notable changes: * Fix for the previous patchset - misplaced comma printing ("dm: Fix comma printing for the case when dm_target_msg structure is inaccessible"). * Update of printstr_ex call, which enables proper handling of QUOTE_0_TERMINATED user style (it prints cropped string without ellipsis otherwise). Eugene Syromyatnikov (9): util: Add support for QUOTE_0_TERMINATED in user_style to ptrintstr_ex tests: Add check for printing of overlength strings to ioctl_dm test tests: Add check for presence of HAVE_LINUX_DM_IOCTL_H macro definition to ioctl_dm test tests/ioctl_dm: whitespace dm: Fix comma printing for the case when dm_target_msg structure is inaccessible tests/ioctl_dm: overly long string printing checks tests: Some additional checks for ioctl_dm test tests: Add ioctl_dm to .gitignore tests: Add checks for abbreviated DM ioctl output dm.c |4 +- tests/.gitignore |2 + tests/Makefile.am |2 + tests/ioctl_dm-v.c|2 + tests/ioctl_dm-v.test | 12 + tests/ioctl_dm.c | 653 +++-- tests/ioctl_dm.test |2 +- util.c| 18 +- 8 files changed, 674 insertions(+), 21 deletions(-) create mode 100644 tests/ioctl_dm-v.c create mode 100755 tests/ioctl_dm-v.test -- 1.7.10.4 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 1/9] util: Add support for QUOTE_0_TERMINATED in user_style to ptrintstr_ex
This enables printing size-limited (expectedly) ASCIZ strings. --- util.c | 18 +++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/util.c b/util.c index 23a5fdb..00148d4 100644 --- a/util.c +++ b/util.c @@ -820,13 +820,13 @@ printstr_ex(struct tcb *tcp, long addr, long len, unsigned int user_style) outstr = xmalloc(outstr_size); } - size = max_strlen; + size = max_strlen + 1; if (len == -1) { /* * Treat as a NUL-terminated string: fetch one byte more * because string_quote may look one byte ahead. */ - if (umovestr(tcp, addr, size + 1, str) < 0) { + if (umovestr(tcp, addr, size, str) < 0) { printaddr(addr); return; } @@ -844,11 +844,23 @@ printstr_ex(struct tcb *tcp, long addr, long len, unsigned int user_style) style |= user_style; + if (style & QUOTE_0_TERMINATED) { + if (size) { + --size; + } else { + tprints((len == -1) || (len == 0) ? "\"\"" : "\"\"..."); + return; + } + } + if (size > max_strlen) + size = max_strlen; + /* If string_quote didn't see NUL and (it was supposed to be ASCIZ str * or we were requested to print more than -s NUM chars)... */ ellipsis = (string_quote(str, outstr, size, style) && - (len < 0 || (unsigned long) len > max_strlen)); + ((style & QUOTE_0_TERMINATED) || + (unsigned long) len > max_strlen)); tprints(outstr); if (ellipsis) -- 1.7.10.4 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 3/9] tests: Add check for presence of HAVE_LINUX_DM_IOCTL_H macro definition to ioctl_dm test
--- tests/ioctl_dm.c | 21 +++-- 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/tests/ioctl_dm.c b/tests/ioctl_dm.c index 0a3bbf4..5f2689c 100644 --- a/tests/ioctl_dm.c +++ b/tests/ioctl_dm.c @@ -1,10 +1,13 @@ #include "tests.h" -#include -#include -#include -#include -#include -#include + +#ifdef HAVE_LINUX_DM_IOCTL_H + +# include +# include +# include +# include +# include +# include static struct s { struct dm_ioctl ioc; @@ -103,3 +106,9 @@ main(void) puts("+++ exited with 0 +++"); return 0; } + +#else /* !HAVE_LINUX_DM_IOCTL_H */ + +SKIP_MAIN_UNDEFINED("HAVE_LINUX_DM_IOCTL_H") + +#endif /* HAVE_LINUX_DM_IOCTL_H */ -- 1.7.10.4 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 5/9] dm: Fix comma printing for the case when dm_target_msg structure is inaccessible
--- dm.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dm.c b/dm.c index d846233..b1d455c 100644 --- a/dm.c +++ b/dm.c @@ -342,10 +342,12 @@ dm_decode_dm_target_msg(struct tcb *tcp, unsigned long addr, offset + target_msg_message_offs <= ioc->data_size) { struct dm_target_msg s; + tprints(", "); + if (umove_or_printaddr(tcp, addr + offset, &s)) return; - tprintf(", {sector=%" PRI__u64 ", message=", s.sector); + tprintf("{sector=%" PRI__u64 ", message=", s.sector); printstr_ex(tcp, addr + offset + target_msg_message_offs, ioc->data_size - offset - target_msg_message_offs, QUOTE_0_TERMINATED); -- 1.7.10.4 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 9/9] tests: Add checks for abbreviated DM ioctl output
--- tests/.gitignore |1 + tests/Makefile.am |2 + tests/ioctl_dm-v.c|2 + tests/ioctl_dm-v.test | 12 tests/ioctl_dm.c | 178 +++-- tests/ioctl_dm.test |2 +- 6 files changed, 160 insertions(+), 37 deletions(-) create mode 100644 tests/ioctl_dm-v.c create mode 100755 tests/ioctl_dm-v.test diff --git a/tests/.gitignore b/tests/.gitignore index 9045117..b2c5434 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -104,6 +104,7 @@ inet-cmsg ioctl ioctl_block ioctl_dm +ioctl_dm-v ioctl_evdev ioctl_evdev-v ioctl_mtd diff --git a/tests/Makefile.am b/tests/Makefile.am index 2405415..61b6db7 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -164,6 +164,7 @@ check_PROGRAMS = \ ioctl \ ioctl_block \ ioctl_dm \ + ioctl_dm-v \ ioctl_evdev \ ioctl_evdev-v \ ioctl_mtd \ @@ -513,6 +514,7 @@ DECODER_TESTS = \ ioctl.test \ ioctl_block.test \ ioctl_dm.test \ + ioctl_dm-v.test \ ioctl_evdev.test \ ioctl_evdev-v.test \ ioctl_mtd.test \ diff --git a/tests/ioctl_dm-v.c b/tests/ioctl_dm-v.c new file mode 100644 index 000..d95058f --- /dev/null +++ b/tests/ioctl_dm-v.c @@ -0,0 +1,2 @@ +#define VERBOSE 1 +#include "ioctl_dm.c" diff --git a/tests/ioctl_dm-v.test b/tests/ioctl_dm-v.test new file mode 100755 index 000..4f6d64c --- /dev/null +++ b/tests/ioctl_dm-v.test @@ -0,0 +1,12 @@ +#!/bin/sh + +# Check abbreviated decoding of DM* ioctls. + +. "${srcdir=.}/init.sh" + +run_prog > /dev/null +run_strace -a16 -s9 -veioctl $args > "$EXP" +check_prog grep +grep -v '^ioctl([012],' < "$LOG" > "$OUT" +match_diff "$OUT" "$EXP" +rm -f "$EXP" "$OUT" diff --git a/tests/ioctl_dm.c b/tests/ioctl_dm.c index 0b2c5a7..2fcd430 100644 --- a/tests/ioctl_dm.c +++ b/tests/ioctl_dm.c @@ -11,6 +11,10 @@ # include # include +# ifndef VERBOSE +# define VERBOSE 0 +# endif + # define STR32 "AbCdEfGhIjKlMnOpQrStUvWxYz012345" static const char str129[] = STR32 STR32 STR32 STR32 "6"; @@ -102,6 +106,7 @@ init_dm_target_spec(struct dm_target_spec *ptr, uint32_t id) ptr->target_type[id % (sizeof(ptr->target_type) + 1)] = '\0'; } +# if VERBOSE static void print_dm_target_spec(struct dm_target_spec *ptr, uint32_t id) { @@ -112,6 +117,7 @@ print_dm_target_spec(struct dm_target_spec *ptr, uint32_t id) (int) (id % (sizeof(ptr->target_type) + 1)), str129 + id % (sizeof(str129) - sizeof(ptr->target_type))); } +# endif /* VERBOSE */ # define ARG_STR(_arg) (_arg), #_arg @@ -303,9 +309,14 @@ main(void) printf("ioctl(-1, DM_TABLE_LOAD, " "{version=4.1.2, data_size=%u, data_start=%u, " "dev=makedev(18, 52), name=\"nnn\", uuid=\"uuu\", " - "target_count=1, flags=0, {sector_start=16, " - "length=32, target_type=\"tgt\", string=\"tparams\"}}) = " - "-1 EBADF (%m)\n", s.ioc.data_size, s.ioc.data_start); + "target_count=1, flags=0, " +# if VERBOSE + "{sector_start=16, length=32, target_type=\"tgt\", " + "string=\"tparams\"}" +# else /* !VERBOSE */ + "..." +# endif /* VERBOSE */ + "}) = -1 EBADF (%m)\n", s.ioc.data_size, s.ioc.data_start); /* No targets */ init_s(dm_arg, sizeof(*dm_arg) - sizeof(dm_arg->data), @@ -328,8 +339,12 @@ main(void) "{version=4.1.2, data_size=%zu, data_start=%u, " "dev=makedev(18, 52), name=\"nnn\", uuid=\"uuu\", " "target_count=1234, flags=0, " - "/* misplaced struct dm_target_spec */ ...}) = -1 EBADF (%m)\n", - sizeof(*dm_arg), 0xfff8); +# if VERBOSE + "/* misplaced struct dm_target_spec */ ..." +# else /* !VERBOSE */ + "..." +# endif /* VERBOSE */ + "}) = -1 EBADF (%m)\n", sizeof(*dm_arg), 0xfff8); /* Inaccessible pointer */ init_s(&dm_arg_open1->ioc, offsetof(struct dm_table_open_test, target1), @@ -340,11 +355,19 @@ main(void) printf("ioctl(-1, DM_TABLE_LOAD, " "{version=4.1.2, data_size=%zu, data_start=%zu, " "dev=makedev(18, 52), name=\"nnn\", uuid=\"uuu\", " - "target_count=3735936673, flags=0, %p}) = -1 EBADF (%m)\n", - sizeof(*dm_arg_open1), - offsetof(struct dm_table_open_test, target1), - (char *) dm_arg_open1 + - offsetof(struct dm_table_open_test, target1)); + "target_count=3735936673, flags=0, " +# if VERBOSE + "%p" +# else /* !VERBOSE */ + "..." +# endif /* VERBOSE */ + "}) = -1 EBADF (%m)\n", sizeof(*dm_arg_open1), + offsetof(struct dm_table_open_test, target1) +# if VERBOSE + , (char *) dm_arg_open1 + + offsetof(struct dm_table_open_test, target1) +# endif /* VERBOSE *
[dm-devel] [PATCH 6/9] tests/ioctl_dm: overly long string printing checks
--- tests/ioctl_dm.c |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/ioctl_dm.c b/tests/ioctl_dm.c index 261983c..24232b7 100644 --- a/tests/ioctl_dm.c +++ b/tests/ioctl_dm.c @@ -67,12 +67,12 @@ main(void) init_s(&s.ioc, sizeof(s), offsetof(struct s, u)); s.u.tm.target_msg.sector = 0x1234; strcpy(s.u.string + offsetof(struct dm_target_msg, message), - "tmsg"); + "long target msg"); ioctl(-1, DM_TARGET_MSG, &s); printf("ioctl(-1, DM_TARGET_MSG, " "{version=4.1.2, data_size=%u, data_start=%u, " "dev=makedev(18, 52), name=\"nnn\", uuid=\"uuu\", flags=0, " - "{sector=4660, message=\"tmsg\"}}) = -1 EBADF (%m)\n", + "{sector=4660, message=\"long targ\"...}}) = -1 EBADF (%m)\n", s.ioc.data_size, s.ioc.data_start); init_s(&s.ioc, sizeof(s), offsetof(struct s, u)); @@ -85,12 +85,12 @@ main(void) s.ioc.data_size, s.ioc.data_start); init_s(&s.ioc, sizeof(s), offsetof(struct s, u)); - strcpy(s.u.string, "new-name"); + strcpy(s.u.string, "new long name"); ioctl(-1, DM_DEV_RENAME, &s); printf("ioctl(-1, DM_DEV_RENAME, " "{version=4.1.2, data_size=%u, data_start=%u, " "dev=makedev(18, 52), name=\"nnn\", uuid=\"uuu\", event_nr=0, " - "flags=0, string=\"new-name\"}) = -1 EBADF (%m)\n", + "flags=0, string=\"new long \"...}) = -1 EBADF (%m)\n", s.ioc.data_size, s.ioc.data_start); init_s(&s.ioc, sizeof(s), offsetof(struct s, u)); -- 1.7.10.4 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel