On 23.01.19 14:31, Vladimir Sementsov-Ogievskiy wrote: > 23.01.2019 16:22, Vladimir Sementsov-Ogievskiy wrote: >> 16.01.2019 16:11, Max Reitz wrote: >>> On 14.01.19 17:06, Vladimir Sementsov-Ogievskiy wrote: >>>> 14.01.2019 17:46, Max Reitz wrote: >>>>> On 29.12.18 13:20, Vladimir Sementsov-Ogievskiy wrote: >>>>>> After node graph changes, we may not be able to resume_drive by device >>>>>> name (backing files are not recursively searched). So, lets allow to >>>>>> resume by node-name. Set constant name for breakpoints, to avoid >>>>>> introducing extra parameters. >>>>> >>>>> Hm, I don't quite understand this reason.  Is this so you can create >>>>> breakpoints on one node (which falls through to the first blkdebug node) >>>>> and then remove them from another (falling through to the same blkdebug >>>>> node)? >>>> >>>> add/remove breakpoint goes through ->file children, but my filter links >>>> active disk as ->backing. So, before block-job start we can insert breakpoint >>>> by device name. But then, when filter inserted, we can't remove breakpoint, >>>> because my filter hides blkdebug with active disk under ->backing link. >>>> >>>> Maybe, right solution would be support backing links in bdrv_debug_breakpoint() >>>> and bdrv_debug_remove_breakpoint() for the case when there is no file child. >>>> >>>> But being unsure about right behavior, I've decided to adjust the test. >>>> >>>> What about just do both  add/remove breakpoint through blkdebug node-name, to >>>> make it less weird? >>> >>> Yes, I was mostly wondering about the "set constant name for >>> breakpoints".  It doesn't seem necessary to me; >> >> It's just to not create extra parameters or something. Current code don't allow >> to create several breakpoints in one blkdebug anyway (as @drive is used as >> breakpoint name), so there is no reason for different names at all. >> >> I'll improve commit message to make it clean. >> > > Hmmm. > > Or, finally, may be better to teach bdrv_debug_breakpoint() and > bdrv_debug_remove_breakpoint() to skip filters with backing child? I suppose that would be a question for my "Deal with filters" series. It does make sense to me, though. Max