I think you're right. We should modify recv_begin_check_existing_impl to fail if fromguid != 0 and force is false. If we don't, we're guaranteed to fail in dsl_dataset_clone_swap_check_impl (unless the dataset has not been modified since the origin, I guess. But given that we (almost?) always modify datasets when we create them, that doesn't seem likely), and that's definitely suboptimal.
As for the ENODEV vs EEXIST issue, after we've changed it to require force, it will only return ENODEV in the case where tofs exists and has snapshots, fromguid == 0, and force is true. I think that ENODEV is technically correct in that case (the latest snapshot of the existing tofs does not map the fromsnap of the stream, namely the origin), though I would be fine with changing it to EEXIST instead. On Wed, Apr 22, 2015 at 5:45 AM, [email protected] wrote: > I wonder if dmu_recv_begin_check() -> recv_begin_check_existing_impl() > should > fail in the following case: > - full stream, fromguid == 0 > - tofs already exists > - tofs does not have any snapshots [*] > - force is false > > In my opinion there is no way that this could succeed. > But right now this seem to succeed only to fail later in > dmu_recv_end_check() > (with ETXTBSY). That seems like suboptimal behavior. > > Also, if tofs has any snapshots then dmu_recv_begin_check() fails. But I > do not > see why the check should succeed even when there are no snapshots. > Additionally, in this case dmu_recv_begin_check() returns ENODEV, which in > this > context is typically interpreted to mean that a source of an incremental > stream > does not match a latest snapshot of a target filesystem. That's a little > bit > confusing since we have a full stream. Maybe it would have better to > return > EEXIST signalling that the target filesystem exists. > -- Paul Dagnelie
_______________________________________________ developer mailing list [email protected] http://lists.open-zfs.org/mailman/listinfo/developer
