Re: [Crash-utility] [PATCH 1/1] CLI list command, print deep structure members

On Tue, Apr 28, 2015 at 09:44:43AM -0400, Dave Anderson wrote: > ----- Original Message ----- > > ----- Original Message ----- > > > Hi Dave, > > > > > > I found it useful to be able listing structure's fields which are > > > resided > > > deeper than the first level: > > > > > > crash> list super_block.s_list -s > > > super_block.s_id,s_dquot.info[1].dqi_dirty_list,s_dquot.dqonoff_mutex.count.counter > > > -H 0xc0a9c800 > > > de805c00 > > > s_id = > > > "sysfs\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000", > > > s_dquot.info[1].dqi_dirty_list = { > > > next = 0x0, > > > prev = 0x0 > > > }, > > > s_dquot.dqonoff_mutex.count.counter = 1 > > > de805800 > > > s_id = > > > "rootfs\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000", > > > s_dquot.info[1].dqi_dirty_list = { > > > next = 0x0, > > > prev = 0x0 > > > }, > > > s_dquot.dqonoff_mutex.count.counter = 1 > > > > > > Here is a draft patch which contains corresponding logic. > > > I will appreciate your comments and suggestions. > > > > > > Best regards, > > > Alexandr > > > > Hi Alexandr, > > > > This looks quite useful. > > > > I haven't looked at the details, but this kind of change modifies an a critical > > area of usage that I cannot risk breaking. > > > > To prevent any kind of possible regression/breakage or other unexpected behavior, > > would it be possible to call parse_for_member_new() *only* if it is required? > > In other words, if the commented-out MEMBER_EXISTS() call fails, then you could > > set a flag to force it to attempt parse_for_member_new(). Otherwise, it could > > continue to use the older/simpler parse_for_member() function. Sounds reasonable. Will do. > > I'll dig into the patch later, test it out, etc., and it's entirely possible > > that I'll change my mind, but paranoia forces me to err on the side of > > caution. > > > > Dave > > Another issue that comes to mind is that there are several other instances > in the crash utility command set besides the "list" command that specify > arguments that can be expressed in the "struct.member" format. > > By any chance, did you also attempt to fit this capability into the "struct" > command itself? And then there's also "tree", "task -R", "waitq", etc. Haven't seen these commands before, will check. However I've found some issue while testing, so please wait with review until I send fixed version.